From 8d7cde22c31a59d0503230334b1d68e858364c9a Mon Sep 17 00:00:00 2001 From: Signal Linden Date: Tue, 11 Oct 2022 15:10:04 -0700 Subject: Replace llbase with llsd module --- indra/llcommon/tests/llleap_test.cpp | 2 +- indra/llcommon/tests/llsdserialize_test.cpp | 2 +- indra/llcorehttp/tests/test_llcorehttp_peer.py | 3 +-- indra/llmessage/tests/test_llsdmessage_peer.py | 3 +-- indra/newview/viewer_manifest.py | 2 +- 5 files changed, 5 insertions(+), 7 deletions(-) (limited to 'indra') diff --git a/indra/llcommon/tests/llleap_test.cpp b/indra/llcommon/tests/llleap_test.cpp index 7ee36a9ea6..60005fc6a9 100644 --- a/indra/llcommon/tests/llleap_test.cpp +++ b/indra/llcommon/tests/llleap_test.cpp @@ -109,7 +109,7 @@ namespace tut "import os\n" "import sys\n" "\n" - "from llbase import llsd\n" + "import llsd\n" "\n" "class ProtocolError(Exception):\n" " def __init__(self, msg, data):\n" diff --git a/indra/llcommon/tests/llsdserialize_test.cpp b/indra/llcommon/tests/llsdserialize_test.cpp index c246f5ee56..be7ec12094 100644 --- a/indra/llcommon/tests/llsdserialize_test.cpp +++ b/indra/llcommon/tests/llsdserialize_test.cpp @@ -1706,7 +1706,7 @@ namespace tut // scanner. import_llsd("import os.path\n" "import sys\n" - "from llbase import llsd\n") + "import llsd\n") {} ~TestPythonCompatible() {} diff --git a/indra/llcorehttp/tests/test_llcorehttp_peer.py b/indra/llcorehttp/tests/test_llcorehttp_peer.py index 778de90962..b9992538ba 100755 --- a/indra/llcorehttp/tests/test_llcorehttp_peer.py +++ b/indra/llcorehttp/tests/test_llcorehttp_peer.py @@ -38,8 +38,7 @@ from io import StringIO from http.server import HTTPServer, BaseHTTPRequestHandler -from llbase.fastest_elementtree import parse as xml_parse -from llbase import llsd +import llsd # we're in llcorehttp/tests ; testrunner.py is found in llmessage/tests sys.path.append(os.path.join(os.path.dirname(__file__), os.pardir, os.pardir, diff --git a/indra/llmessage/tests/test_llsdmessage_peer.py b/indra/llmessage/tests/test_llsdmessage_peer.py index 5ba0749e31..ff8f40a144 100755 --- a/indra/llmessage/tests/test_llsdmessage_peer.py +++ b/indra/llmessage/tests/test_llsdmessage_peer.py @@ -33,8 +33,7 @@ import os import sys from http.server import HTTPServer, BaseHTTPRequestHandler -from llbase.fastest_elementtree import parse as xml_parse -from llbase import llsd +import llsd from testrunner import freeport, run, debug, VERBOSE import time diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index 3327ecfb56..30b0d23a93 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -49,7 +49,7 @@ viewer_dir = os.path.dirname(__file__) # indra.util.llmanifest under their system Python! sys.path.insert(0, os.path.join(viewer_dir, os.pardir, "lib", "python")) from indra.util.llmanifest import LLManifest, main, path_ancestors, CHANNEL_VENDOR_BASE, RELEASE_CHANNEL, ManifestError, MissingError -from llbase import llsd +import llsd class ViewerManifest(LLManifest): def is_packaging_viewer(self): -- cgit v1.2.3 From 6112e92b61c334d3c0656bacdea92690cea1a76b Mon Sep 17 00:00:00 2001 From: Signal Linden Date: Wed, 12 Oct 2022 11:49:47 -0700 Subject: Upload installer, build Release, use large runner --- .../installers/windows/installer_template.nsi | 1 - indra/newview/viewer_manifest.py | 34 ++++++---------------- 2 files changed, 9 insertions(+), 26 deletions(-) (limited to 'indra') diff --git a/indra/newview/installers/windows/installer_template.nsi b/indra/newview/installers/windows/installer_template.nsi index 7513908cb4..f7908b059c 100644 --- a/indra/newview/installers/windows/installer_template.nsi +++ b/indra/newview/installers/windows/installer_template.nsi @@ -26,7 +26,6 @@ ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; Compiler flags ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; -Unicode true SetOverwrite on # Overwrite files SetCompress auto # Compress if saves space SetCompressor /solid lzma # Compress whole installer as one block diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index 30b0d23a93..eec8e84a2d 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -28,7 +28,6 @@ $/LicenseInfo$ """ import errno import glob -import itertools import json import os import os.path @@ -36,12 +35,9 @@ import plistlib import random import re import shutil -import stat import subprocess import sys -import tarfile import time -import zipfile viewer_dir = os.path.dirname(__file__) # Add indra/lib/python to our path so we don't have to muck with PYTHONPATH. @@ -789,27 +785,15 @@ class WindowsManifest(ViewerManifest): # Check two paths, one for Program Files, and one for Program Files (x86). # Yay 64bit windows. - for ProgramFiles in 'ProgramFiles', 'ProgramFiles(x86)': - NSIS_path = os.path.expandvars(r'${%s}\NSIS\makensis.exe' % ProgramFiles) - if os.path.exists(NSIS_path): - break - installer_created=False - nsis_attempts=3 - nsis_retry_wait=15 - for attempt in range(nsis_attempts): - try: - self.run_command([NSIS_path, '/V2', self.dst_path_of(tempfile)]) - except ManifestError as err: - if attempt+1 < nsis_attempts: - print("nsis failed, waiting %d seconds before retrying" % nsis_retry_wait, file=sys.stderr) - time.sleep(nsis_retry_wait) - nsis_retry_wait*=2 - else: - # NSIS worked! Done! - break - else: - print("Maximum nsis attempts exceeded; giving up", file=sys.stderr) - raise + nsis_path = "makensis.exe" + for program_files in '${programfiles}', '${programfiles(x86)}': + for nesis_path in 'NSIS', 'NSIS\\Unicode': + possible_path = os.path.expandvars(f"{program_files}\\{nesis_path}\\makensis.exe") + if os.path.exists(possible_path): + nsis_path = possible_path + break + + self.run_command([possible_path, '/V2', self.dst_path_of(tempfile)]) self.sign(installer_file) self.created_path(self.dst_path_of(installer_file)) -- cgit v1.2.3 From efb987f595d6749d42e3148bf0e071f5bafce450 Mon Sep 17 00:00:00 2001 From: Signal Linden Date: Wed, 12 Oct 2022 12:59:12 -0700 Subject: Run on windows-2022-large --- indra/cmake/00-Common.cmake | 9 --------- 1 file changed, 9 deletions(-) (limited to 'indra') diff --git a/indra/cmake/00-Common.cmake b/indra/cmake/00-Common.cmake index c848d00710..bf5a107c73 100644 --- a/indra/cmake/00-Common.cmake +++ b/indra/cmake/00-Common.cmake @@ -55,15 +55,6 @@ if (WINDOWS) # http://www.cmake.org/pipermail/cmake/2009-September/032143.html string(REPLACE "/Zm1000" " " CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS}) - # Without PreferredToolArchitecture=x64, as of 2020-06-26 the 32-bit - # compiler on our TeamCity build hosts has started running out of virtual - # memory for the precompiled header file. - # CP changed to only append the flag for 32bit builds - on 64bit builds, - # locally at least, the build output is spammed with 1000s of 'D9002' - # warnings about this switch being ignored. - if( ADDRESS_SIZE EQUAL 32 ) - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /p:PreferredToolArchitecture=x64") - endif() # zlib has assembly-language object files incompatible with SAFESEH add_link_options(/LARGEADDRESSAWARE /SAFESEH:NO -- cgit v1.2.3 From 53c89d9723b816d06516ae59d7e0f0d12e477ec9 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 2 Jun 2023 16:30:28 -0400 Subject: SL-18837: Don't try to copy long, specific libnghttp2.14.19.0.dylib. The package doesn't include that any more. --- indra/cmake/Copy3rdPartyLibs.cmake | 1 - 1 file changed, 1 deletion(-) (limited to 'indra') diff --git a/indra/cmake/Copy3rdPartyLibs.cmake b/indra/cmake/Copy3rdPartyLibs.cmake index d43cc30706..e4d06d6ba2 100644 --- a/indra/cmake/Copy3rdPartyLibs.cmake +++ b/indra/cmake/Copy3rdPartyLibs.cmake @@ -172,7 +172,6 @@ elseif(DARWIN) libndofdev.dylib libnghttp2.dylib libnghttp2.14.dylib - libnghttp2.14.19.0.dylib liburiparser.dylib liburiparser.1.dylib liburiparser.1.0.27.dylib -- cgit v1.2.3 From 6d2d0c8ee59eed641937b19cf19a1ff08762cbfa Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 2 Jun 2023 17:27:57 -0400 Subject: SL-18837: Boost.Bind _1, _2 placeholders are no longer global. This was a longstanding complaint: that Boost shouldn't dump the (somewhat mysterious) _1, _2 et al. names into the global namespace. Recent Boost has fixed that, requiring 'using namespace boost::placeholders;' if you want to use them unqualified. --- indra/llinventory/llsettingsbase.cpp | 4 ++++ indra/llinventory/llsettingssky.cpp | 4 ++++ indra/llinventory/llsettingswater.cpp | 4 ++++ 3 files changed, 12 insertions(+) (limited to 'indra') diff --git a/indra/llinventory/llsettingsbase.cpp b/indra/llinventory/llsettingsbase.cpp index 936b166409..ba338dbbee 100644 --- a/indra/llinventory/llsettingsbase.cpp +++ b/indra/llinventory/llsettingsbase.cpp @@ -31,6 +31,10 @@ #include #include "llsdserialize.h" +#include + +// allow unqualified _1, _2 et al. to mean boost::bind placeholders +using namespace boost::placeholders; //========================================================================= namespace diff --git a/indra/llinventory/llsettingssky.cpp b/indra/llinventory/llsettingssky.cpp index a129f0a6f0..0244b2585e 100644 --- a/indra/llinventory/llsettingssky.cpp +++ b/indra/llinventory/llsettingssky.cpp @@ -31,6 +31,10 @@ #include "lltrace.h" #include "llfasttimer.h" #include "v3colorutil.h" +#include + +// allow unqualified _1, _2 et al. to mean boost::bind placeholders +using namespace boost::placeholders; //========================================================================= namespace diff --git a/indra/llinventory/llsettingswater.cpp b/indra/llinventory/llsettingswater.cpp index d732032a6c..bc53a46255 100644 --- a/indra/llinventory/llsettingswater.cpp +++ b/indra/llinventory/llsettingswater.cpp @@ -32,6 +32,10 @@ #include "llfasttimer.h" #include "v3colorutil.h" #include "indra_constants.h" +#include + +// allow unqualified _1, _2 et al. to mean boost::bind placeholders +using namespace boost::placeholders; const std::string LLSettingsWater::SETTING_BLUR_MULTIPLIER("blur_multiplier"); const std::string LLSettingsWater::SETTING_FOG_COLOR("water_fog_color"); -- cgit v1.2.3 From 42c69448da85b06caf9edabb4dd3f5f4ae926ea7 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 2 Jun 2023 17:32:31 -0400 Subject: SL-18837: Don't include a version-specific Boost.Regex header. Let the umbrella header make that decision. --- indra/newview/lllogchat.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra') diff --git a/indra/newview/lllogchat.cpp b/indra/newview/lllogchat.cpp index ba82ff0b0f..8c03292361 100644 --- a/indra/newview/lllogchat.cpp +++ b/indra/newview/lllogchat.cpp @@ -41,7 +41,7 @@ #include #include -#include +#include #include #if LL_MSVC -- cgit v1.2.3 From a4a68c4f5e2e411ba636d25e6b07129ca5de2bb9 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 5 Jun 2023 10:39:34 -0400 Subject: SL-18837: We no longer build Windows apr-iconv -- don't reference. With VS 2022 on Windows GitHub Actions runners, we can't build apr_suite at all with the upstream .sln / .vcxproj files, so we had to switch to "experimental" CMake support. However there's no CMakeLists.txt file for apr-iconv, so the Windows package omits that library. --- indra/cmake/APR.cmake | 2 -- indra/cmake/Copy3rdPartyLibs.cmake | 1 - 2 files changed, 3 deletions(-) (limited to 'indra') diff --git a/indra/cmake/APR.cmake b/indra/cmake/APR.cmake index 8a0939c92c..21139319c3 100644 --- a/indra/cmake/APR.cmake +++ b/indra/cmake/APR.cmake @@ -16,7 +16,6 @@ if (WINDOWS) endif (LLCOMMON_LINK_SHARED) target_link_libraries( ll::apr INTERFACE ${ARCH_PREBUILT_DIRS_RELEASE}/${APR_selector}apr-1.lib - ${ARCH_PREBUILT_DIRS_RELEASE}/${APR_selector}apriconv-1.lib ${ARCH_PREBUILT_DIRS_RELEASE}/${APR_selector}aprutil-1.lib ) elseif (DARWIN) @@ -37,7 +36,6 @@ else (WINDOWS) target_link_libraries( ll::apr INTERFACE apr-1 aprutil-1 - iconv uuid rt ) diff --git a/indra/cmake/Copy3rdPartyLibs.cmake b/indra/cmake/Copy3rdPartyLibs.cmake index e4d06d6ba2..a3db02372d 100644 --- a/indra/cmake/Copy3rdPartyLibs.cmake +++ b/indra/cmake/Copy3rdPartyLibs.cmake @@ -57,7 +57,6 @@ if(WINDOWS) openjp2.dll libapr-1.dll libaprutil-1.dll - libapriconv-1.dll nghttp2.dll libhunspell.dll uriparser.dll -- cgit v1.2.3 From 524f3b2af08c4e2a856a70d4505391c6f56d76df Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 5 Jun 2023 10:43:16 -0400 Subject: SL-18837: #include in several sources that need it. It seems we're no longer implicitly inheriting from some other [set of] header file[s]. Where we use std::array, bring it in explicitly. --- indra/llaudio/llaudioengine.h | 1 + indra/newview/llenvironment.h | 2 ++ indra/newview/llimview.cpp | 2 ++ 3 files changed, 5 insertions(+) (limited to 'indra') diff --git a/indra/llaudio/llaudioengine.h b/indra/llaudio/llaudioengine.h index 0fe8b3d756..cf694cfaf4 100755 --- a/indra/llaudio/llaudioengine.h +++ b/indra/llaudio/llaudioengine.h @@ -30,6 +30,7 @@ #include #include +#include #include "v3math.h" #include "v3dmath.h" diff --git a/indra/newview/llenvironment.h b/indra/newview/llenvironment.h index 64fd170e43..1c8a68ae02 100644 --- a/indra/newview/llenvironment.h +++ b/indra/newview/llenvironment.h @@ -42,6 +42,8 @@ #include +#include + //------------------------------------------------------------------------- class LLViewerCamera; class LLParcel; diff --git a/indra/newview/llimview.cpp b/indra/newview/llimview.cpp index 6880cf2171..61a01d7418 100644 --- a/indra/newview/llimview.cpp +++ b/indra/newview/llimview.cpp @@ -72,6 +72,8 @@ #include "llcorehttputil.h" #include "lluiusage.h" +#include + const static std::string ADHOC_NAME_SUFFIX(" Conference"); const static std::string NEARBY_P2P_BY_OTHER("nearby_P2P_by_other"); -- cgit v1.2.3 From 045342ba29aae186e13c711bd4dd84377d4a7e43 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 5 Jun 2023 11:28:41 -0400 Subject: SL-18837: Bump the granularity of WorkQueue timing tests. On a low-powered GitHub Mac runner, the system doesn't wake up as soon as it should, and we get spurious "too late" errors. Try a bigger time increment. --- indra/llcommon/tests/workqueue_test.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'indra') diff --git a/indra/llcommon/tests/workqueue_test.cpp b/indra/llcommon/tests/workqueue_test.cpp index 1d73f7aa0d..7655a7aa1f 100644 --- a/indra/llcommon/tests/workqueue_test.cpp +++ b/indra/llcommon/tests/workqueue_test.cpp @@ -83,7 +83,11 @@ namespace tut // signal the work item that it can quit; consider LLOneShotCond. LLCond data; auto start = WorkQueue::TimePoint::clock::now(); - auto interval = 100ms; + // 2s seems like a long time to wait, since it directly impacts the + // duration of this test program. Unfortunately GitHub's Mac runners + // are pretty wimpy, and we're getting spurious "too late" errors just + // because the thread doesn't wake up as soon as we want. + auto interval = 2s; queue.postEvery( interval, [&data, count = 0] -- cgit v1.2.3 From b5e1484c00880bfd8001cd1418f3eb96fc38b89e Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 5 Jun 2023 12:22:28 -0400 Subject: SL-18837: Windows APR 1.7.2 requires MS rpcrt4.dll. --- indra/cmake/Linking.cmake | 1 + 1 file changed, 1 insertion(+) (limited to 'indra') diff --git a/indra/cmake/Linking.cmake b/indra/cmake/Linking.cmake index 4a501f420b..1ce21c11f9 100644 --- a/indra/cmake/Linking.cmake +++ b/indra/cmake/Linking.cmake @@ -62,6 +62,7 @@ elseif (WINDOWS) user32 ole32 dbghelp + rpcrt4.lib legacy_stdio_definitions ) else() -- cgit v1.2.3 From 19e9e8cf419c18b527bc19c5da1ab2cdf040dd0a Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 5 Jun 2023 12:32:10 -0400 Subject: SL-18837: Try to silence cascade of Boost.Bind warning messages. --- indra/cmake/00-Common.cmake | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'indra') diff --git a/indra/cmake/00-Common.cmake b/indra/cmake/00-Common.cmake index bf5a107c73..dc9ccd356a 100644 --- a/indra/cmake/00-Common.cmake +++ b/indra/cmake/00-Common.cmake @@ -26,6 +26,11 @@ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} $ENV{LL_BUILD}") # Portable compilation flags. add_compile_definitions( ADDRESS_SIZE=${ADDRESS_SIZE}) +# Because older versions of Boost.Bind dumped placeholders _1, _2 et al. into +# the global namespace, Boost now requires either BOOST_BIND_NO_PLACEHOLDERS +# to avoid that or BOOST_BIND_GLOBAL_PLACEHOLDERS to state that we require it +# -- which we do. Without one or the other, we get a ton of Boost warnings. +add_compile_definitions(BOOST_BIND_GLOBAL_PLACEHOLDERS) # Configure crash reporting set(RELEASE_CRASH_REPORTING OFF CACHE BOOL "Enable use of crash reporting in release builds") -- cgit v1.2.3 From ddc6d219585251ea6246f56bfa5aba6b3e2fd4f0 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 5 Jun 2023 21:47:52 -0400 Subject: SL-18837: Followup to 19e9e8c: global Boost.Bind placeholders do not need 'using' directive, given BOOST_BIND_GLOBAL_PLACEHOLDERS. --- indra/llinventory/llsettingsbase.cpp | 3 --- indra/llinventory/llsettingssky.cpp | 3 --- indra/llinventory/llsettingswater.cpp | 3 --- 3 files changed, 9 deletions(-) (limited to 'indra') diff --git a/indra/llinventory/llsettingsbase.cpp b/indra/llinventory/llsettingsbase.cpp index ba338dbbee..6ea93e045d 100644 --- a/indra/llinventory/llsettingsbase.cpp +++ b/indra/llinventory/llsettingsbase.cpp @@ -33,9 +33,6 @@ #include "llsdserialize.h" #include -// allow unqualified _1, _2 et al. to mean boost::bind placeholders -using namespace boost::placeholders; - //========================================================================= namespace { diff --git a/indra/llinventory/llsettingssky.cpp b/indra/llinventory/llsettingssky.cpp index 0244b2585e..51fca76518 100644 --- a/indra/llinventory/llsettingssky.cpp +++ b/indra/llinventory/llsettingssky.cpp @@ -33,9 +33,6 @@ #include "v3colorutil.h" #include -// allow unqualified _1, _2 et al. to mean boost::bind placeholders -using namespace boost::placeholders; - //========================================================================= namespace { diff --git a/indra/llinventory/llsettingswater.cpp b/indra/llinventory/llsettingswater.cpp index bc53a46255..89156000b0 100644 --- a/indra/llinventory/llsettingswater.cpp +++ b/indra/llinventory/llsettingswater.cpp @@ -34,9 +34,6 @@ #include "indra_constants.h" #include -// allow unqualified _1, _2 et al. to mean boost::bind placeholders -using namespace boost::placeholders; - const std::string LLSettingsWater::SETTING_BLUR_MULTIPLIER("blur_multiplier"); const std::string LLSettingsWater::SETTING_FOG_COLOR("water_fog_color"); const std::string LLSettingsWater::SETTING_FOG_DENSITY("water_fog_density"); -- cgit v1.2.3 From 6914ff6113f1667308293fe37e1ba4a4dcba850f Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 5 Jun 2023 21:51:28 -0400 Subject: SL-18837: Use Boost.Filesystem for NamedTempFile, instead of APR. --- indra/test/namedtempfile.h | 136 +++++++++++---------------------------------- indra/test/test.cpp | 10 ++-- 2 files changed, 38 insertions(+), 108 deletions(-) (limited to 'indra') diff --git a/indra/test/namedtempfile.h b/indra/test/namedtempfile.h index 7d59cad32c..927e2b4fd9 100644 --- a/indra/test/namedtempfile.h +++ b/indra/test/namedtempfile.h @@ -13,15 +13,15 @@ #define LL_NAMEDTEMPFILE_H #include "llerror.h" -#include "llapr.h" -#include "apr_file_io.h" +#include "stringize.h" #include -#include -#include -#include #include +#include +#include +#include #include #include +#include /** * Create a text file with specified content "somewhere in the @@ -31,41 +31,36 @@ class NamedTempFile: public boost::noncopyable { LOG_CLASS(NamedTempFile); public: - NamedTempFile(const std::string& pfx, const std::string& content, apr_pool_t* pool=gAPRPoolp): - mPool(pool) + NamedTempFile(const std::string_view& pfx, + const std::string_view& content, + const std::string_view& sfx=std::string_view("")) { - createFile(pfx, boost::phoenix::placeholders::arg1 << content); - } - - // Disambiguate when passing string literal - NamedTempFile(const std::string& pfx, const char* content, apr_pool_t* pool=gAPRPoolp): - mPool(pool) - { - createFile(pfx, boost::phoenix::placeholders::arg1 << content); + createFile(pfx, [&content](std::ostream& out){ out << content; }, sfx); } // Function that accepts an ostream ref and (presumably) writes stuff to // it, e.g.: // (boost::phoenix::placeholders::arg1 << "the value is " << 17 << '\n') - typedef boost::function Streamer; + typedef std::function Streamer; - NamedTempFile(const std::string& pfx, const Streamer& func, apr_pool_t* pool=gAPRPoolp): - mPool(pool) + NamedTempFile(const std::string_view& pfx, + const Streamer& func, + const std::string_view& sfx=std::string_view("")) { - createFile(pfx, func); + createFile(pfx, func, sfx); } virtual ~NamedTempFile() { - ll_apr_assert_status(apr_file_remove(mPath.c_str(), mPool)); + boost::filesystem::remove(mPath); } - virtual std::string getName() const { return mPath; } + virtual std::string getName() const { return mPath.native(); } void peep() { std::cout << "File '" << mPath << "' contains:\n"; - std::ifstream reader(mPath.c_str()); + boost::filesystem::ifstream reader(mPath); std::string line; while (std::getline(reader, line)) std::cout << line << '\n'; @@ -73,92 +68,40 @@ public: } protected: - void createFile(const std::string& pfx, const Streamer& func) + void createFile(const std::string_view& pfx, + const Streamer& func, + const std::string_view& sfx) { // Create file in a temporary place. - const char* tempdir = NULL; - ll_apr_assert_status(apr_temp_dir_get(&tempdir, mPool)); - - // Construct a temp filename template in that directory. - char *tempname = NULL; - ll_apr_assert_status(apr_filepath_merge(&tempname, - tempdir, - (pfx + "XXXXXX").c_str(), - 0, - mPool)); - - // Create a temp file from that template. - apr_file_t* fp = NULL; - ll_apr_assert_status(apr_file_mktemp(&fp, - tempname, - APR_CREATE | APR_WRITE | APR_EXCL, - mPool)); - // apr_file_mktemp() alters tempname with the actual name. Not until - // now is it valid to capture as our mPath. - mPath = tempname; + boost::filesystem::path tempname{ boost::filesystem::temp_directory_path() }; + // unique_path() recommended model + tempname += stringize(pfx, "%%%%-%%%%-%%%%-%%%%", sfx); + mPath = boost::filesystem::unique_path(tempname); + boost::filesystem::ofstream out{ mPath }; // Write desired content. - std::ostringstream out; - // Stream stuff to it. func(out); - - std::string data(out.str()); - apr_size_t writelen(data.length()); - ll_apr_assert_status(apr_file_write(fp, data.c_str(), &writelen)); - ll_apr_assert_status(apr_file_close(fp)); - llassert_always(writelen == data.length()); } - std::string mPath; - apr_pool_t* mPool; + boost::filesystem::path mPath; }; /** * Create a NamedTempFile with a specified filename extension. This is useful * when, for instance, you must be able to use the file in a Python import * statement. - * - * A NamedExtTempFile actually has two different names. We retain the original - * no-extension name as a placeholder in the temp directory to ensure - * uniqueness; to that we link the name plus the desired extension. Naturally, - * both must be removed on destruction. */ class NamedExtTempFile: public NamedTempFile { LOG_CLASS(NamedExtTempFile); public: - NamedExtTempFile(const std::string& ext, const std::string& content, apr_pool_t* pool=gAPRPoolp): - NamedTempFile(remove_dot(ext), content, pool), - mLink(mPath + ensure_dot(ext)) - { - linkto(mLink); - } + NamedExtTempFile(const std::string& ext, const std::string_view& content): + NamedTempFile(remove_dot(ext), content, ensure_dot(ext)) + {} - // Disambiguate when passing string literal - NamedExtTempFile(const std::string& ext, const char* content, apr_pool_t* pool=gAPRPoolp): - NamedTempFile(remove_dot(ext), content, pool), - mLink(mPath + ensure_dot(ext)) - { - linkto(mLink); - } - - NamedExtTempFile(const std::string& ext, const Streamer& func, apr_pool_t* pool=gAPRPoolp): - NamedTempFile(remove_dot(ext), func, pool), - mLink(mPath + ensure_dot(ext)) - { - linkto(mLink); - } - - virtual ~NamedExtTempFile() - { - ll_apr_assert_status(apr_file_remove(mLink.c_str(), mPool)); - } - - // Since the caller has gone to the trouble to create the name with the - // extension, that should be the name we return. In this class, mPath is - // just a placeholder to ensure that future createFile() calls won't - // collide. - virtual std::string getName() const { return mLink; } + NamedExtTempFile(const std::string& ext, const Streamer& func): + NamedTempFile(remove_dot(ext), func, ensure_dot(ext)) + {} static std::string ensure_dot(const std::string& ext) { @@ -175,7 +118,7 @@ public: { return ext; } - return std::string(".") + ext; + return "." + ext; } static std::string remove_dot(const std::string& ext) @@ -187,19 +130,6 @@ public: } return ext.substr(found); } - -private: - void linkto(const std::string& path) - { - // This method assumes that since mPath (without extension) is - // guaranteed by apr_file_mktemp() to be unique, then (mPath + any - // extension) is also unique. This is likely, though not guaranteed: - // files could be created in the same temp directory other than by - // this class. - ll_apr_assert_status(apr_file_link(mPath.c_str(), path.c_str())); - } - - std::string mLink; }; #endif /* ! defined(LL_NAMEDTEMPFILE_H) */ diff --git a/indra/test/test.cpp b/indra/test/test.cpp index bb48216b2b..04f32831b7 100644 --- a/indra/test/test.cpp +++ b/indra/test/test.cpp @@ -97,10 +97,10 @@ public: class RecordToTempFile : public LLError::Recorder, public boost::noncopyable { public: - RecordToTempFile(apr_pool_t* pPool) + RecordToTempFile() : LLError::Recorder(), boost::noncopyable(), - mTempFile("log", "", pPool), + mTempFile("log", ""), mFile(mTempFile.getName().c_str()) { } @@ -141,11 +141,11 @@ private: class LLReplayLogReal: public LLReplayLog, public boost::noncopyable { public: - LLReplayLogReal(LLError::ELevel level, apr_pool_t* pool) + LLReplayLogReal(LLError::ELevel level) : LLReplayLog(), boost::noncopyable(), mOldSettings(LLError::saveAndResetSettings()), - mRecorder(new RecordToTempFile(pool)) + mRecorder(new RecordToTempFile()) { LLError::setFatalFunction(wouldHaveCrashed); LLError::setDefaultLevel(level); @@ -624,7 +624,7 @@ int main(int argc, char **argv) if (LOGFAIL && *LOGFAIL) { LLError::ELevel level = LLError::decodeLevel(LOGFAIL); - replayer.reset(new LLReplayLogReal(level, gAPRPoolp)); + replayer.reset(new LLReplayLogReal(level)); } } LLError::setFatalFunction(wouldHaveCrashed); -- cgit v1.2.3 From ca510f6c299335c8db27c65c10a8553801c06023 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 5 Jun 2023 22:08:26 -0400 Subject: SL-18837: Try giving temp Python scripts a .py extension. On GitHub Windows Actions runners, we're getting permissions errors trying to tell the Python interpreter to run a NamedTempFile script. Try using NamedExtTempFile to give each such script a .py extension. --- indra/llcommon/tests/llleap_test.cpp | 222 ++++++++++++++++---------------- indra/llcommon/tests/llprocess_test.cpp | 4 +- 2 files changed, 113 insertions(+), 113 deletions(-) (limited to 'indra') diff --git a/indra/llcommon/tests/llleap_test.cpp b/indra/llcommon/tests/llleap_test.cpp index 60005fc6a9..99fd073dd2 100644 --- a/indra/llcommon/tests/llleap_test.cpp +++ b/indra/llcommon/tests/llleap_test.cpp @@ -213,9 +213,9 @@ namespace tut void object::test<1>() { set_test_name("multiple LLLeap instances"); - NamedTempFile script("py", - "import time\n" - "time.sleep(1)\n"); + NamedExtTempFile script("py", + "import time\n" + "time.sleep(1)\n"); LLLeapVector instances; instances.push_back(LLLeap::create(get_test_name(), sv(list_of(PYTHON)(script.getName())))->getWeak()); @@ -231,10 +231,10 @@ namespace tut void object::test<2>() { set_test_name("stderr to log"); - NamedTempFile script("py", - "import sys\n" - "sys.stderr.write('''Hello from Python!\n" - "note partial line''')\n"); + NamedExtTempFile script("py", + "import sys\n" + "sys.stderr.write('''Hello from Python!\n" + "note partial line''')\n"); StringVec vcommand{ PYTHON, script.getName() }; CaptureLog log(LLError::LEVEL_INFO); waitfor(LLLeap::create(get_test_name(), vcommand)); @@ -246,8 +246,8 @@ namespace tut void object::test<3>() { set_test_name("bad stdout protocol"); - NamedTempFile script("py", - "print('Hello from Python!')\n"); + NamedExtTempFile script("py", + "print('Hello from Python!')\n"); CaptureLog log(LLError::LEVEL_WARN); waitfor(LLLeap::create(get_test_name(), sv(list_of(PYTHON)(script.getName())))); @@ -259,10 +259,10 @@ namespace tut void object::test<4>() { set_test_name("leftover stdout"); - NamedTempFile script("py", - "import sys\n" - // note lack of newline - "sys.stdout.write('Hello from Python!')\n"); + NamedExtTempFile script("py", + "import sys\n" + // note lack of newline + "sys.stdout.write('Hello from Python!')\n"); CaptureLog log(LLError::LEVEL_WARN); waitfor(LLLeap::create(get_test_name(), sv(list_of(PYTHON)(script.getName())))); @@ -274,9 +274,9 @@ namespace tut void object::test<5>() { set_test_name("bad stdout len prefix"); - NamedTempFile script("py", - "import sys\n" - "sys.stdout.write('5a2:something')\n"); + NamedExtTempFile script("py", + "import sys\n" + "sys.stdout.write('5a2:something')\n"); CaptureLog log(LLError::LEVEL_WARN); waitfor(LLLeap::create(get_test_name(), sv(list_of(PYTHON)(script.getName())))); @@ -381,16 +381,16 @@ namespace tut set_test_name("round trip"); AckAPI api; Result result; - NamedTempFile script("py", - boost::phoenix::placeholders::arg1 << - "from " << reader_module << " import *\n" - // make a request on our little API - "request(pump='" << api.getName() << "', data={})\n" - // wait for its response - "resp = get()\n" - "result = '' if resp == dict(pump=replypump(), data='ack')\\\n" - " else 'bad: ' + str(resp)\n" - "send(pump='" << result.getName() << "', data=result)\n"); + NamedExtTempFile script("py", + boost::phoenix::placeholders::arg1 << + "from " << reader_module << " import *\n" + // make a request on our little API + "request(pump='" << api.getName() << "', data={})\n" + // wait for its response + "resp = get()\n" + "result = '' if resp == dict(pump=replypump(), data='ack')\\\n" + " else 'bad: ' + str(resp)\n" + "send(pump='" << result.getName() << "', data=result)\n"); waitfor(LLLeap::create(get_test_name(), sv(list_of(PYTHON)(script.getName())))); result.ensure(); } @@ -419,37 +419,37 @@ namespace tut // iterations etc. in OS pipes and the LLLeap/LLProcess implementation. ReqIDAPI api; Result result; - NamedTempFile script("py", - boost::phoenix::placeholders::arg1 << - "import sys\n" - "from " << reader_module << " import *\n" - // Note that since reader imports llsd, this - // 'import *' gets us llsd too. - "sample = llsd.format_notation(dict(pump='" << - api.getName() << "', data=dict(reqid=999999, reply=replypump())))\n" - // The whole packet has length prefix too: "len:data" - "samplen = len(str(len(sample))) + 1 + len(sample)\n" - // guess how many messages it will take to - // accumulate BUFFERED_LENGTH - "count = int(" << BUFFERED_LENGTH << "/samplen)\n" - "print('Sending %s requests' % count, file=sys.stderr)\n" - "for i in range(count):\n" - " request('" << api.getName() << "', dict(reqid=i))\n" - // The assumption in this specific test that - // replies will arrive in the same order as - // requests is ONLY valid because the API we're - // invoking sends replies instantly. If the API - // had to wait for some external event before - // sending its reply, replies could arrive in - // arbitrary order, and we'd have to tick them - // off from a set. - "result = ''\n" - "for i in range(count):\n" - " resp = get()\n" - " if resp['data']['reqid'] != i:\n" - " result = 'expected reqid=%s in %s' % (i, resp)\n" - " break\n" - "send(pump='" << result.getName() << "', data=result)\n"); + NamedExtTempFile script("py", + boost::phoenix::placeholders::arg1 << + "import sys\n" + "from " << reader_module << " import *\n" + // Note that since reader imports llsd, this + // 'import *' gets us llsd too. + "sample = llsd.format_notation(dict(pump='" << + api.getName() << "', data=dict(reqid=999999, reply=replypump())))\n" + // The whole packet has length prefix too: "len:data" + "samplen = len(str(len(sample))) + 1 + len(sample)\n" + // guess how many messages it will take to + // accumulate BUFFERED_LENGTH + "count = int(" << BUFFERED_LENGTH << "/samplen)\n" + "print('Sending %s requests' % count, file=sys.stderr)\n" + "for i in range(count):\n" + " request('" << api.getName() << "', dict(reqid=i))\n" + // The assumption in this specific test that + // replies will arrive in the same order as + // requests is ONLY valid because the API we're + // invoking sends replies instantly. If the API + // had to wait for some external event before + // sending its reply, replies could arrive in + // arbitrary order, and we'd have to tick them + // off from a set. + "result = ''\n" + "for i in range(count):\n" + " resp = get()\n" + " if resp['data']['reqid'] != i:\n" + " result = 'expected reqid=%s in %s' % (i, resp)\n" + " break\n" + "send(pump='" << result.getName() << "', data=result)\n"); waitfor(LLLeap::create(get_test_name(), sv(list_of(PYTHON)(script.getName()))), 300); // needs more realtime than most tests result.ensure(); @@ -462,60 +462,60 @@ namespace tut { ReqIDAPI api; Result result; - NamedTempFile script("py", - boost::phoenix::placeholders::arg1 << - "import sys\n" - "from " << reader_module << " import *\n" - // Generate a very large string value. - "desired = int(sys.argv[1])\n" - // 7 chars per item: 6 digits, 1 comma - "count = int((desired - 50)/7)\n" - "large = ''.join('%06d,' % i for i in range(count))\n" - // Pass 'large' as reqid because we know the API - // will echo reqid, and we want to receive it back. - "request('" << api.getName() << "', dict(reqid=large))\n" - "try:\n" - " resp = get()\n" - "except ParseError as e:\n" - " # try to find where e.data diverges from expectation\n" - // Normally we'd expect a 'pump' key in there, - // too, with value replypump(). But Python - // serializes keys in a different order than C++, - // so incoming data start with 'data'. - // Truthfully, though, if we get as far as 'pump' - // before we find a difference, something's very - // strange. - " expect = llsd.format_notation(dict(data=dict(reqid=large)))\n" - " chunk = 40\n" - " for offset in range(0, max(len(e.data), len(expect)), chunk):\n" - " if e.data[offset:offset+chunk] != \\\n" - " expect[offset:offset+chunk]:\n" - " print('Offset %06d: expect %r,\\n'\\\n" - " ' get %r' %\\\n" - " (offset,\n" - " expect[offset:offset+chunk],\n" - " e.data[offset:offset+chunk]),\n" - " file=sys.stderr)\n" - " break\n" - " else:\n" - " print('incoming data matches expect?!', file=sys.stderr)\n" - " send('" << result.getName() << "', '%s: %s' % (e.__class__.__name__, e))\n" - " sys.exit(1)\n" - "\n" - "echoed = resp['data']['reqid']\n" - "if echoed == large:\n" - " send('" << result.getName() << "', '')\n" - " sys.exit(0)\n" - // Here we know echoed did NOT match; try to find where - "for i in range(count):\n" - " start = 7*i\n" - " end = 7*(i+1)\n" - " if end > len(echoed)\\\n" - " or echoed[start:end] != large[start:end]:\n" - " send('" << result.getName() << "',\n" - " 'at offset %s, expected %r but got %r' %\n" - " (start, large[start:end], echoed[start:end]))\n" - "sys.exit(1)\n"); + NamedExtTempFile script("py", + boost::phoenix::placeholders::arg1 << + "import sys\n" + "from " << reader_module << " import *\n" + // Generate a very large string value. + "desired = int(sys.argv[1])\n" + // 7 chars per item: 6 digits, 1 comma + "count = int((desired - 50)/7)\n" + "large = ''.join('%06d,' % i for i in range(count))\n" + // Pass 'large' as reqid because we know the API + // will echo reqid, and we want to receive it back. + "request('" << api.getName() << "', dict(reqid=large))\n" + "try:\n" + " resp = get()\n" + "except ParseError as e:\n" + " # try to find where e.data diverges from expectation\n" + // Normally we'd expect a 'pump' key in there, + // too, with value replypump(). But Python + // serializes keys in a different order than C++, + // so incoming data start with 'data'. + // Truthfully, though, if we get as far as 'pump' + // before we find a difference, something's very + // strange. + " expect = llsd.format_notation(dict(data=dict(reqid=large)))\n" + " chunk = 40\n" + " for offset in range(0, max(len(e.data), len(expect)), chunk):\n" + " if e.data[offset:offset+chunk] != \\\n" + " expect[offset:offset+chunk]:\n" + " print('Offset %06d: expect %r,\\n'\\\n" + " ' get %r' %\\\n" + " (offset,\n" + " expect[offset:offset+chunk],\n" + " e.data[offset:offset+chunk]),\n" + " file=sys.stderr)\n" + " break\n" + " else:\n" + " print('incoming data matches expect?!', file=sys.stderr)\n" + " send('" << result.getName() << "', '%s: %s' % (e.__class__.__name__, e))\n" + " sys.exit(1)\n" + "\n" + "echoed = resp['data']['reqid']\n" + "if echoed == large:\n" + " send('" << result.getName() << "', '')\n" + " sys.exit(0)\n" + // Here we know echoed did NOT match; try to find where + "for i in range(count):\n" + " start = 7*i\n" + " end = 7*(i+1)\n" + " if end > len(echoed)\\\n" + " or echoed[start:end] != large[start:end]:\n" + " send('" << result.getName() << "',\n" + " 'at offset %s, expected %r but got %r' %\n" + " (start, large[start:end], echoed[start:end]))\n" + "sys.exit(1)\n"); waitfor(LLLeap::create(test_name, sv(list_of (PYTHON) diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index 81449b4a42..4adb8d872a 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -191,7 +191,7 @@ struct PythonProcessLauncher LLProcess::Params mParams; LLProcessPtr mPy; std::string mDesc; - NamedTempFile mScript; + NamedExtTempFile mScript; }; /// convenience function for PythonProcessLauncher::run() @@ -355,7 +355,7 @@ namespace tut set_test_name("raw APR nonblocking I/O"); // Create a script file in a temporary place. - NamedTempFile script("py", + NamedExtTempFile script("py", "from __future__ import print_function" EOL "import sys" EOL "import time" EOL -- cgit v1.2.3 From aab769e9d7a9b11256572f8adb69d586d9ff8d3d Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 5 Jun 2023 23:24:34 -0400 Subject: SL-18837: Make NamedTempFile revert to boost::function from std::function, since some consumers still use (e.g.) boost::phoenix::placeholders::arg1 to generate an inline callable. --- indra/test/namedtempfile.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'indra') diff --git a/indra/test/namedtempfile.h b/indra/test/namedtempfile.h index 927e2b4fd9..df7495fc13 100644 --- a/indra/test/namedtempfile.h +++ b/indra/test/namedtempfile.h @@ -15,10 +15,10 @@ #include "llerror.h" #include "stringize.h" #include -#include +#include #include #include -#include +#include #include #include #include @@ -41,7 +41,7 @@ public: // Function that accepts an ostream ref and (presumably) writes stuff to // it, e.g.: // (boost::phoenix::placeholders::arg1 << "the value is " << 17 << '\n') - typedef std::function Streamer; + typedef boost::function Streamer; NamedTempFile(const std::string_view& pfx, const Streamer& func, -- cgit v1.2.3 From 26ca3e14d623e4094dde76ad88e3da2a209483b5 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 5 Jun 2023 23:46:43 -0400 Subject: SL-18837: NamedTempFile must still disambiguate string literals. --- indra/test/namedtempfile.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'indra') diff --git a/indra/test/namedtempfile.h b/indra/test/namedtempfile.h index df7495fc13..acfc048b7a 100644 --- a/indra/test/namedtempfile.h +++ b/indra/test/namedtempfile.h @@ -38,6 +38,15 @@ public: createFile(pfx, [&content](std::ostream& out){ out << content; }, sfx); } + // Disambiguate when passing string literal -- unclear why a string + // literal should be ambiguous wrt std::string_view and Streamer + NamedTempFile(const std::string_view& pfx, + const char* content, + const std::string_view& sfx=std::string_view("")) + { + createFile(pfx, [&content](std::ostream& out){ out << content; }, sfx); + } + // Function that accepts an ostream ref and (presumably) writes stuff to // it, e.g.: // (boost::phoenix::placeholders::arg1 << "the value is " << 17 << '\n') @@ -99,6 +108,11 @@ public: NamedTempFile(remove_dot(ext), content, ensure_dot(ext)) {} + // Disambiguate when passing string literal + NamedExtTempFile(const std::string& ext, const char* content): + NamedTempFile(remove_dot(ext), content, ensure_dot(ext)) + {} + NamedExtTempFile(const std::string& ext, const Streamer& func): NamedTempFile(remove_dot(ext), func, ensure_dot(ext)) {} -- cgit v1.2.3 From 6516c9d07db42beba5ba9c0c41a33925794a249c Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 6 Jun 2023 07:44:42 -0400 Subject: SL-18837: NamedTempFile back to std::function, use boost::phoenix << It seems the problem addressed by aab769e wasn't some synergy between Boost.Phoenix and Boost.Function, but rather the lack of a Phoenix header file introducing operator<<(). --- indra/llcommon/tests/llleap_test.cpp | 1 + indra/llcommon/tests/llsdserialize_test.cpp | 1 + indra/test/namedtempfile.h | 4 ++-- 3 files changed, 4 insertions(+), 2 deletions(-) (limited to 'indra') diff --git a/indra/llcommon/tests/llleap_test.cpp b/indra/llcommon/tests/llleap_test.cpp index 99fd073dd2..0c91db3e24 100644 --- a/indra/llcommon/tests/llleap_test.cpp +++ b/indra/llcommon/tests/llleap_test.cpp @@ -19,6 +19,7 @@ // external library headers #include #include +#include // operator<<() // other Linden headers #include "../test/lltut.h" #include "../test/namedtempfile.h" diff --git a/indra/llcommon/tests/llsdserialize_test.cpp b/indra/llcommon/tests/llsdserialize_test.cpp index d7c11c5021..a0b8519508 100644 --- a/indra/llcommon/tests/llsdserialize_test.cpp +++ b/indra/llcommon/tests/llsdserialize_test.cpp @@ -50,6 +50,7 @@ typedef U32 uint32_t; #include "boost/bind.hpp" #include "boost/phoenix/bind/bind_function.hpp" #include "boost/phoenix/core/argument.hpp" +#include using namespace boost::phoenix; #include "../llsd.h" diff --git a/indra/test/namedtempfile.h b/indra/test/namedtempfile.h index acfc048b7a..84b62a0945 100644 --- a/indra/test/namedtempfile.h +++ b/indra/test/namedtempfile.h @@ -15,10 +15,10 @@ #include "llerror.h" #include "stringize.h" #include -#include #include #include #include +#include #include #include #include @@ -50,7 +50,7 @@ public: // Function that accepts an ostream ref and (presumably) writes stuff to // it, e.g.: // (boost::phoenix::placeholders::arg1 << "the value is " << 17 << '\n') - typedef boost::function Streamer; + typedef std::function Streamer; NamedTempFile(const std::string_view& pfx, const Streamer& func, -- cgit v1.2.3 From 1db7ac7139adf505be12308fd7ba2920f5beecde Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 6 Jun 2023 10:02:57 -0400 Subject: SL-18837: Make NamedTempFile name template compatible with Python. The recommended template uses hyphens; change to underscores to be valid Python temp module names. --- indra/test/namedtempfile.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'indra') diff --git a/indra/test/namedtempfile.h b/indra/test/namedtempfile.h index 84b62a0945..8cd6c990dc 100644 --- a/indra/test/namedtempfile.h +++ b/indra/test/namedtempfile.h @@ -83,8 +83,9 @@ protected: { // Create file in a temporary place. boost::filesystem::path tempname{ boost::filesystem::temp_directory_path() }; - // unique_path() recommended model - tempname += stringize(pfx, "%%%%-%%%%-%%%%-%%%%", sfx); + // unique_path() recommended template, but with underscores instead of + // hyphens: some use cases involve temporary Python scripts + tempname += stringize(pfx, "%%%%_%%%%_%%%%_%%%%", sfx); mPath = boost::filesystem::unique_path(tempname); boost::filesystem::ofstream out{ mPath }; -- cgit v1.2.3 From c4366378b61cacb9d87eb2917302fa17e9207491 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 6 Jun 2023 10:04:26 -0400 Subject: SL-18837: Ditch Boost.Phoenix implicit lambda syntax. It's cool to be able to write 'arg1 << "stuff" << var ...;' for a lambda accepting a std::ostream reference, but cascading compile errors mean it's no longer worth trying to make that work -- given actual C++ lambdas. Also clean up a lingering BOOST_FOREACH() and a boost::bind() while at it. --- indra/llcommon/tests/llleap_test.cpp | 18 ++++++++---------- indra/llcommon/tests/llsdserialize_test.cpp | 21 +++++++-------------- 2 files changed, 15 insertions(+), 24 deletions(-) (limited to 'indra') diff --git a/indra/llcommon/tests/llleap_test.cpp b/indra/llcommon/tests/llleap_test.cpp index 0c91db3e24..0fc741d9e1 100644 --- a/indra/llcommon/tests/llleap_test.cpp +++ b/indra/llcommon/tests/llleap_test.cpp @@ -18,8 +18,6 @@ #include // external library headers #include -#include -#include // operator<<() // other Linden headers #include "../test/lltut.h" #include "../test/namedtempfile.h" @@ -105,7 +103,7 @@ namespace tut llleap_data(): reader(".py", // This logic is adapted from vita.viewerclient.receiveEvent() - boost::phoenix::placeholders::arg1 << + [](std::ostream& out){ out << "import re\n" "import os\n" "import sys\n" @@ -189,7 +187,7 @@ namespace tut "def request(pump, data):\n" " # we expect 'data' is a dict\n" " data['reply'] = _reply\n" - " send(pump, data)\n"), + " send(pump, data)\n";}), // Get the actual pathname of the NamedExtTempFile and trim off // the ".py" extension. (We could cache reader.getName() in a // separate member variable, but I happen to know getName() just @@ -383,7 +381,7 @@ namespace tut AckAPI api; Result result; NamedExtTempFile script("py", - boost::phoenix::placeholders::arg1 << + [&](std::ostream& out){ out << "from " << reader_module << " import *\n" // make a request on our little API "request(pump='" << api.getName() << "', data={})\n" @@ -391,7 +389,7 @@ namespace tut "resp = get()\n" "result = '' if resp == dict(pump=replypump(), data='ack')\\\n" " else 'bad: ' + str(resp)\n" - "send(pump='" << result.getName() << "', data=result)\n"); + "send(pump='" << result.getName() << "', data=result)\n";}); waitfor(LLLeap::create(get_test_name(), sv(list_of(PYTHON)(script.getName())))); result.ensure(); } @@ -421,7 +419,7 @@ namespace tut ReqIDAPI api; Result result; NamedExtTempFile script("py", - boost::phoenix::placeholders::arg1 << + [&](std::ostream& out){ out << "import sys\n" "from " << reader_module << " import *\n" // Note that since reader imports llsd, this @@ -450,7 +448,7 @@ namespace tut " if resp['data']['reqid'] != i:\n" " result = 'expected reqid=%s in %s' % (i, resp)\n" " break\n" - "send(pump='" << result.getName() << "', data=result)\n"); + "send(pump='" << result.getName() << "', data=result)\n";}); waitfor(LLLeap::create(get_test_name(), sv(list_of(PYTHON)(script.getName()))), 300); // needs more realtime than most tests result.ensure(); @@ -464,7 +462,7 @@ namespace tut ReqIDAPI api; Result result; NamedExtTempFile script("py", - boost::phoenix::placeholders::arg1 << + [&](std::ostream& out){ out << "import sys\n" "from " << reader_module << " import *\n" // Generate a very large string value. @@ -516,7 +514,7 @@ namespace tut " send('" << result.getName() << "',\n" " 'at offset %s, expected %r but got %r' %\n" " (start, large[start:end], echoed[start:end]))\n" - "sys.exit(1)\n"); + "sys.exit(1)\n";}); waitfor(LLLeap::create(test_name, sv(list_of (PYTHON) diff --git a/indra/llcommon/tests/llsdserialize_test.cpp b/indra/llcommon/tests/llsdserialize_test.cpp index a0b8519508..08bf7fbc51 100644 --- a/indra/llcommon/tests/llsdserialize_test.cpp +++ b/indra/llcommon/tests/llsdserialize_test.cpp @@ -45,13 +45,6 @@ typedef U32 uint32_t; #endif #include "boost/range.hpp" -#include "boost/foreach.hpp" -#include "boost/function.hpp" -#include "boost/bind.hpp" -#include "boost/phoenix/bind/bind_function.hpp" -#include "boost/phoenix/core/argument.hpp" -#include -using namespace boost::phoenix; #include "../llsd.h" #include "../llsdserialize.h" @@ -1802,7 +1795,7 @@ namespace tut // helper for test<3> static void writeLLSDArray(std::ostream& out, const LLSD& array) { - BOOST_FOREACH(LLSD item, llsd::inArray(array)) + for (const LLSD& item: llsd::inArray(array)) { LLSDSerialize::toNotation(item, out); // It's important to separate with newlines because Python's llsd @@ -1842,21 +1835,21 @@ namespace tut // Create an llsdXXXXXX file containing 'data' serialized to // notation. NamedTempFile file("llsd", - // NamedTempFile's boost::function constructor + // NamedTempFile's function constructor // takes a callable. To this callable it passes the // std::ostream with which it's writing the // NamedTempFile. - boost::bind(writeLLSDArray, _1, cdata)); + [&](std::ostream& out){ writeLLSDArray(out, cdata); }); python("read C++ notation", - placeholders::arg1 << + [&](std::ostream& out){ out << import_llsd << "def parse_each(iterable):\n" " for item in iterable:\n" " yield llsd.parse(item)\n" << pydata << // Don't forget raw-string syntax for Windows pathnames. - "verify(parse_each(open(r'" << file.getName() << "', 'rb')))\n"); + "verify(parse_each(open(r'" << file.getName() << "', 'rb')))\n";}); } template<> template<> @@ -1870,7 +1863,7 @@ namespace tut NamedTempFile file("llsd", ""); python("write Python notation", - placeholders::arg1 << + [&](std::ostream& out){ out << import_llsd << "DATA = [\n" " 17,\n" @@ -1884,7 +1877,7 @@ namespace tut // N.B. Using 'print' implicitly adds newlines. "with open(r'" << file.getName() << "', 'w') as f:\n" " for item in DATA:\n" - " print(llsd.format_notation(item).decode(), file=f)\n"); + " print(llsd.format_notation(item).decode(), file=f)\n";}); std::ifstream inf(file.getName().c_str()); LLSD item; -- cgit v1.2.3 From e63c571d1336e3c521e1fc3a5e27bb77fc667790 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 6 Jun 2023 13:34:01 -0400 Subject: SL-18837: On Windows, NamedTempFile must convert from wstring --- indra/test/namedtempfile.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'indra') diff --git a/indra/test/namedtempfile.h b/indra/test/namedtempfile.h index 8cd6c990dc..da4fec97c8 100644 --- a/indra/test/namedtempfile.h +++ b/indra/test/namedtempfile.h @@ -13,6 +13,7 @@ #define LL_NAMEDTEMPFILE_H #include "llerror.h" +#include "llstring.h" #include "stringize.h" #include #include @@ -64,7 +65,8 @@ public: boost::filesystem::remove(mPath); } - virtual std::string getName() const { return mPath.native(); } + // On Windows, path::native() returns a wstring + std::string getName() const { return ll_convert(mPath.native()); } void peep() { -- cgit v1.2.3 From 004150a5305d0df06c52a51a0df3ac26dd4a63cd Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 6 Jun 2023 16:27:43 -0400 Subject: SL-18837: Concat path part with / rather than +=. Using concatenation appends the intended filename to the parent directory name, instead of putting the filename in the parent directory. --- indra/test/namedtempfile.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'indra') diff --git a/indra/test/namedtempfile.h b/indra/test/namedtempfile.h index da4fec97c8..c215c50f3f 100644 --- a/indra/test/namedtempfile.h +++ b/indra/test/namedtempfile.h @@ -84,10 +84,12 @@ protected: const std::string_view& sfx) { // Create file in a temporary place. - boost::filesystem::path tempname{ boost::filesystem::temp_directory_path() }; - // unique_path() recommended template, but with underscores instead of - // hyphens: some use cases involve temporary Python scripts - tempname += stringize(pfx, "%%%%_%%%%_%%%%_%%%%", sfx); + boost::filesystem::path tempname{ + boost::filesystem::temp_directory_path() / + // unique_path() recommended template, but with underscores + // instead of hyphens: some use cases involve temporary Python + // scripts + stringize(pfx, "%%%%_%%%%_%%%%_%%%%", sfx) }; mPath = boost::filesystem::unique_path(tempname); boost::filesystem::ofstream out{ mPath }; -- cgit v1.2.3 From f57de07f73871bc7be6c338ea18893a494d104eb Mon Sep 17 00:00:00 2001 From: Brad Linden Date: Wed, 7 Jun 2023 13:46:29 -0700 Subject: Attempt to port some build.yaml improvements from DRTVWR-559 over to actions branch --- indra/cmake/Variables.cmake | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) (limited to 'indra') diff --git a/indra/cmake/Variables.cmake b/indra/cmake/Variables.cmake index e6285ab48b..778384f87c 100644 --- a/indra/cmake/Variables.cmake +++ b/indra/cmake/Variables.cmake @@ -173,13 +173,17 @@ if (${CMAKE_SYSTEM_NAME} MATCHES "Darwin") set(CMAKE_XCODE_ATTRIBUTE_GCC_OPTIMIZATION_LEVEL "${CMAKE_MATCH_1}") message(STATUS "CMAKE_XCODE_ATTRIBUTE_GCC_OPTIMIZATION_LEVEL = '${CMAKE_XCODE_ATTRIBUTE_GCC_OPTIMIZATION_LEVEL}'") - string(REGEX MATCHALL "[^ ]+" LL_BUILD_LIST "$ENV{LL_BUILD}") - list(FIND LL_BUILD_LIST "-iwithsysroot" sysroot_idx) - if ("${sysroot_idx}" LESS 0) - message(FATAL_ERROR "Environment variable LL_BUILD must contain '-iwithsysroot'") - endif () - math(EXPR sysroot_idx "${sysroot_idx} + 1") - list(GET LL_BUILD_LIST "${sysroot_idx}" CMAKE_OSX_SYSROOT) + # allow disabling this check by setting LL_SKIP_REQUIRE_SYSROOT either ON as cmake cache var or non-empty as environment var + set(LL_SKIP_REQUIRE_SYSROOT OFF CACHE BOOL "Skip requirement to set toolchain sysroot ahead of time. Not skipped by default for consistency, but skipping can be useful for selecting alternative xcode versions side by side") + if("$ENV{LL_SKIP_REQUIRE_SYSROOT}" STREQUAL "" AND NOT ${LL_SKIP_REQUIRE_SYSROOT}) + string(REGEX MATCHALL "[^ ]+" LL_BUILD_LIST "$ENV{LL_BUILD}") + list(FIND LL_BUILD_LIST "-iwithsysroot" sysroot_idx) + if ("${sysroot_idx}" LESS 0) + message(FATAL_ERROR "Environment variable LL_BUILD must contain '-iwithsysroot'") + endif () + math(EXPR sysroot_idx "${sysroot_idx} + 1") + list(GET LL_BUILD_LIST "${sysroot_idx}" CMAKE_OSX_SYSROOT) + endif() message(STATUS "CMAKE_OSX_SYSROOT = '${CMAKE_OSX_SYSROOT}'") set(CMAKE_XCODE_ATTRIBUTE_GCC_VERSION "com.apple.compilers.llvm.clang.1_0") -- cgit v1.2.3 From 3c63cc9c55b1193f2876e7523f0b6766f5a8c5ac Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 28 Jun 2023 18:36:51 -0400 Subject: SL-18837: Make LLVersionInfo::getBuild() S64 for GitHub run IDs. --- indra/newview/llappviewer.cpp | 4 ++-- indra/newview/llpanellogin.cpp | 4 ++-- indra/newview/lltranslate.cpp | 4 ++-- indra/newview/llversioninfo.cpp | 2 +- indra/newview/llversioninfo.h | 2 +- indra/newview/llweb.cpp | 2 +- indra/newview/llxmlrpctransaction.cpp | 2 +- 7 files changed, 10 insertions(+), 10 deletions(-) (limited to 'indra') diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index 8235e4466c..99ace834c2 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -3552,7 +3552,7 @@ void LLAppViewer::writeSystemInfo() gDebugInfo["ClientInfo"]["MajorVersion"] = LLVersionInfo::instance().getMajor(); gDebugInfo["ClientInfo"]["MinorVersion"] = LLVersionInfo::instance().getMinor(); gDebugInfo["ClientInfo"]["PatchVersion"] = LLVersionInfo::instance().getPatch(); - gDebugInfo["ClientInfo"]["BuildVersion"] = LLVersionInfo::instance().getBuild(); + gDebugInfo["ClientInfo"]["BuildVersion"] = std::to_string(LLVersionInfo::instance().getBuild()); gDebugInfo["ClientInfo"]["AddressSize"] = LLVersionInfo::instance().getAddressSize(); gDebugInfo["CAFilename"] = gDirUtilp->getCAFile(); @@ -5669,7 +5669,7 @@ void LLAppViewer::handleLoginComplete() gDebugInfo["ClientInfo"]["MajorVersion"] = LLVersionInfo::instance().getMajor(); gDebugInfo["ClientInfo"]["MinorVersion"] = LLVersionInfo::instance().getMinor(); gDebugInfo["ClientInfo"]["PatchVersion"] = LLVersionInfo::instance().getPatch(); - gDebugInfo["ClientInfo"]["BuildVersion"] = LLVersionInfo::instance().getBuild(); + gDebugInfo["ClientInfo"]["BuildVersion"] = std::to_string(LLVersionInfo::instance().getBuild()); LLParcel* parcel = LLViewerParcelMgr::getInstance()->getAgentParcel(); if ( parcel && parcel->getMusicURL()[0]) diff --git a/indra/newview/llpanellogin.cpp b/indra/newview/llpanellogin.cpp index b14fdbf38e..c61c176530 100644 --- a/indra/newview/llpanellogin.cpp +++ b/indra/newview/llpanellogin.cpp @@ -300,7 +300,7 @@ LLPanelLogin::LLPanelLogin(const LLRect &rect, setDefaultBtn(def_btn); std::string channel = LLVersionInfo::instance().getChannel(); - std::string version = llformat("%s (%d)", + std::string version = llformat("%s (%ld)", LLVersionInfo::instance().getShortVersion().c_str(), LLVersionInfo::instance().getBuild()); @@ -894,7 +894,7 @@ void LLPanelLogin::loadLoginPage() } // Channel and Version - params["version"] = llformat("%s (%d)", + params["version"] = llformat("%s (%ld)", LLVersionInfo::instance().getShortVersion().c_str(), LLVersionInfo::instance().getBuild()); params["channel"] = LLVersionInfo::instance().getChannel(); diff --git a/indra/newview/lltranslate.cpp b/indra/newview/lltranslate.cpp index 34d3ed8bb1..d7cb12826f 100644 --- a/indra/newview/lltranslate.cpp +++ b/indra/newview/lltranslate.cpp @@ -133,7 +133,7 @@ void LLTranslationAPIHandler::verifyKeyCoro(LLTranslate::EService service, std:: LLCore::HttpHeaders::ptr_t httpHeaders(new LLCore::HttpHeaders); - std::string user_agent = llformat("%s %d.%d.%d (%d)", + std::string user_agent = llformat("%s %d.%d.%d (%ld)", LLVersionInfo::instance().getChannel().c_str(), LLVersionInfo::instance().getMajor(), LLVersionInfo::instance().getMinor(), @@ -177,7 +177,7 @@ void LLTranslationAPIHandler::translateMessageCoro(LanguagePair_t fromTo, std::s LLCore::HttpHeaders::ptr_t httpHeaders(new LLCore::HttpHeaders); - std::string user_agent = llformat("%s %d.%d.%d (%d)", + std::string user_agent = llformat("%s %d.%d.%d (%ld)", LLVersionInfo::instance().getChannel().c_str(), LLVersionInfo::instance().getMajor(), LLVersionInfo::instance().getMinor(), diff --git a/indra/newview/llversioninfo.cpp b/indra/newview/llversioninfo.cpp index 376a7fce76..62bfa24e29 100644 --- a/indra/newview/llversioninfo.cpp +++ b/indra/newview/llversioninfo.cpp @@ -91,7 +91,7 @@ S32 LLVersionInfo::getPatch() return LL_VIEWER_VERSION_PATCH; } -S32 LLVersionInfo::getBuild() +S64 LLVersionInfo::getBuild() { return LL_VIEWER_VERSION_BUILD; } diff --git a/indra/newview/llversioninfo.h b/indra/newview/llversioninfo.h index 02ff0c094a..122bd5c47a 100644 --- a/indra/newview/llversioninfo.h +++ b/indra/newview/llversioninfo.h @@ -61,7 +61,7 @@ public: S32 getPatch(); /// return the build number as an integer - S32 getBuild(); + S64 getBuild(); /// return the full viewer version as a string like "2.0.0.200030" std::string getVersion(); diff --git a/indra/newview/llweb.cpp b/indra/newview/llweb.cpp index c4d873dd22..9afe332025 100644 --- a/indra/newview/llweb.cpp +++ b/indra/newview/llweb.cpp @@ -160,7 +160,7 @@ std::string LLWeb::expandURLSubstitutions(const std::string &url, substitution["VERSION_MAJOR"] = LLVersionInfo::instance().getMajor(); substitution["VERSION_MINOR"] = LLVersionInfo::instance().getMinor(); substitution["VERSION_PATCH"] = LLVersionInfo::instance().getPatch(); - substitution["VERSION_BUILD"] = LLVersionInfo::instance().getBuild(); + substitution["VERSION_BUILD"] = std::to_string(LLVersionInfo::instance().getBuild()); substitution["CHANNEL"] = LLVersionInfo::instance().getChannel(); substitution["GRID"] = LLGridManager::getInstance()->getGridId(); substitution["GRID_LOWERCASE"] = utf8str_tolower(LLGridManager::getInstance()->getGridId()); diff --git a/indra/newview/llxmlrpctransaction.cpp b/indra/newview/llxmlrpctransaction.cpp index 8d178dbbdc..b851b7ad5c 100644 --- a/indra/newview/llxmlrpctransaction.cpp +++ b/indra/newview/llxmlrpctransaction.cpp @@ -384,7 +384,7 @@ void LLXMLRPCTransaction::Impl::init(XMLRPC_REQUEST request, bool useGzip, const httpHeaders->append(HTTP_OUT_HEADER_CONTENT_TYPE, HTTP_CONTENT_TEXT_XML); - std::string user_agent = llformat("%s %d.%d.%d (%d)", + std::string user_agent = llformat("%s %d.%d.%d (%ld)", LLVersionInfo::instance().getChannel().c_str(), LLVersionInfo::instance().getMajor(), LLVersionInfo::instance().getMinor(), -- cgit v1.2.3 From f54c1215676f26480d88b4588bb0eeb9c05f50d9 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 7 Jul 2023 10:06:02 -0400 Subject: SL-18837: Try putting generated Python scripts in RUNNER_TEMP dir. The claim is that the Windows Python interpreter is integrated somehow with the OS such that a command line that tries to run Python with a script that "looks suspicious" (i.e. in a system temp directory) fails with "Access denied" without even loading the interpreter. At least that theory would explain the "Access denied" errors we've been getting trying to run Python scripts generated into the system temp directory by our integration tests. Our hope is that generating such scripts into the GitHub RUNNER_TEMP directory will work better. As this test is specific to Windows, don't even bother running Mac builds. --- indra/test/namedtempfile.h | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) (limited to 'indra') diff --git a/indra/test/namedtempfile.h b/indra/test/namedtempfile.h index c215c50f3f..4343361f41 100644 --- a/indra/test/namedtempfile.h +++ b/indra/test/namedtempfile.h @@ -84,12 +84,19 @@ protected: const std::string_view& sfx) { // Create file in a temporary place. + // This variable is set by GitHub actions and is the recommended place + // to put temp files belonging to an actions job. + const char* RUNNER_TEMP = getenv("RUNNER_TEMP"); + boost::filesystem::path tempdir{ + // if RUNNER_TEMP is set and not empty + (RUNNER_TEMP && *RUNNER_TEMP)? + boost::filesystem::path(RUNNER_TEMP) : // use RUNNER_TEMP if available + boost::filesystem::temp_directory_path()}; // else canonical temp dir boost::filesystem::path tempname{ - boost::filesystem::temp_directory_path() / - // unique_path() recommended template, but with underscores - // instead of hyphens: some use cases involve temporary Python - // scripts - stringize(pfx, "%%%%_%%%%_%%%%_%%%%", sfx) }; + // use filename template recommended by unique_path() doc, but + // with underscores instead of hyphens: some use cases involve + // temporary Python scripts + tempdir / stringize(pfx, "%%%%_%%%%_%%%%_%%%%", sfx) }; mPath = boost::filesystem::unique_path(tempname); boost::filesystem::ofstream out{ mPath }; -- cgit v1.2.3 From e933ace53b24b732d4111169e3c5964a8591a29e Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 7 Jul 2023 14:07:12 -0400 Subject: SL-18837: Try to bypass Windows perm problem with Python indirection. --- indra/llcommon/tests/llprocess_test.cpp | 41 ++++++++++++++++++------------ indra/llcommon/tests/test_python_script.py | 20 +++++++++++++++ indra/test/namedtempfile.h | 22 +++++++++------- 3 files changed, 58 insertions(+), 25 deletions(-) create mode 100644 indra/llcommon/tests/test_python_script.py (limited to 'indra') diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index 4adb8d872a..af99e97d66 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -124,6 +124,17 @@ void waitfor(LLProcess::handle h, const std::string& desc, int timeout=60) i < timeout); } +namespace { + +// find test helper, a sibling of this file +// nat 2023-07-07: we're currently using Boost 1.81, but +// path::replace_filename() (which is exactly what we need here) doesn't +// arrive until Boost 1.82. +auto test_python_script{ + (boost::filesystem::path(__FILE__).remove_filename() / "test_python_script.py").string() }; + +} + /** * Construct an LLProcess to run a Python script. */ @@ -145,6 +156,7 @@ struct PythonProcessLauncher mParams.desc = desc + " script"; mParams.executable = PYTHON; + mParams.args.add(test_python_script); mParams.args.add(mScript.getName()); } @@ -214,30 +226,26 @@ static std::string python_out(const std::string& desc, const CONTENT& script) class NamedTempDir: public boost::noncopyable { public: - // Use python() function to create a temp directory: I've found - // nothing in either Boost.Filesystem or APR quite like Python's - // tempfile.mkdtemp(). - // Special extra bonus: on Mac, mkdtemp() reports a pathname - // starting with /var/folders/something, whereas that's really a - // symlink to /private/var/folders/something. Have to use - // realpath() to compare properly. NamedTempDir(): - mPath(python_out("mkdtemp()", - "from __future__ import with_statement\n" - "import os.path, sys, tempfile\n" - "with open(sys.argv[1], 'w') as f:\n" - " f.write(os.path.normcase(os.path.normpath(os.path.realpath(tempfile.mkdtemp()))))\n")) - {} + mPath(NamedTempFile::temp_path()), + mCreated(boost::filesystem::create_directories(mPath)) + { + mPath = boost::filesystem::canonical(mPath); + } ~NamedTempDir() { - aprchk(apr_dir_remove(mPath.c_str(), gAPRPoolp)); + if (mCreated) + { + boost::filesystem::remove_all(mPath); + } } - std::string getName() const { return mPath; } + std::string getName() const { return mPath.string(); } private: - std::string mPath; + boost::filesystem::path mPath; + bool mCreated; }; /***************************************************************************** @@ -390,6 +398,7 @@ namespace tut // Have to have a named copy of this std::string so its c_str() value // will persist. std::string scriptname(script.getName()); + argv.push_back(test_python_script.c_str()); argv.push_back(scriptname.c_str()); argv.push_back(NULL); diff --git a/indra/llcommon/tests/test_python_script.py b/indra/llcommon/tests/test_python_script.py new file mode 100644 index 0000000000..c0c8661aa9 --- /dev/null +++ b/indra/llcommon/tests/test_python_script.py @@ -0,0 +1,20 @@ +#!/usr/bin/env python3 +"""\ +@file test_python_script.py +@author Nat Goodspeed +@date 2023-07-07 +@brief Work around a problem running Python within integration tests on GitHub + Windows runners. + +$LicenseInfo:firstyear=2023&license=viewerlgpl$ +Copyright (c) 2023, Linden Research, Inc. +$/LicenseInfo$ +""" + +import os +import sys + +# use pop() so that if the referenced script in turn looks at sys.argv, it +# will see its arguments rather than its own filename +_script = sys.argv.pop(1) +exec(open(_script).read()) diff --git a/indra/test/namedtempfile.h b/indra/test/namedtempfile.h index 4343361f41..525a35000d 100644 --- a/indra/test/namedtempfile.h +++ b/indra/test/namedtempfile.h @@ -65,8 +65,7 @@ public: boost::filesystem::remove(mPath); } - // On Windows, path::native() returns a wstring - std::string getName() const { return ll_convert(mPath.native()); } + std::string getName() const { return mPath.string(); } void peep() { @@ -78,12 +77,9 @@ public: std::cout << "---\n"; } -protected: - void createFile(const std::string_view& pfx, - const Streamer& func, - const std::string_view& sfx) + static boost::filesystem::path temp_path(const std::string_view& pfx="", + const std::string_view& sfx="") { - // Create file in a temporary place. // This variable is set by GitHub actions and is the recommended place // to put temp files belonging to an actions job. const char* RUNNER_TEMP = getenv("RUNNER_TEMP"); @@ -97,9 +93,17 @@ protected: // with underscores instead of hyphens: some use cases involve // temporary Python scripts tempdir / stringize(pfx, "%%%%_%%%%_%%%%_%%%%", sfx) }; - mPath = boost::filesystem::unique_path(tempname); - boost::filesystem::ofstream out{ mPath }; + return boost::filesystem::unique_path(tempname); + } +protected: + void createFile(const std::string_view& pfx, + const Streamer& func, + const std::string_view& sfx) + { + // Create file in a temporary place. + mPath = temp_path(pfx, sfx); + boost::filesystem::ofstream out{ mPath }; // Write desired content. func(out); } -- cgit v1.2.3 From c4b5d089dad5680a0dd12b2d386b692318eb5c58 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 7 Jul 2023 16:57:20 -0400 Subject: SL-18837: Partially revert e933ace, keeping useful tweaks. Introducing indirection via test_python_script.py did NOT address the "Access is denied" errors on GitHub Windows runners. --- indra/llcommon/tests/llleap_test.cpp | 25 +++++++++---------------- indra/llcommon/tests/llprocess_test.cpp | 13 ------------- indra/llcommon/tests/test_python_script.py | 20 -------------------- 3 files changed, 9 insertions(+), 49 deletions(-) delete mode 100644 indra/llcommon/tests/test_python_script.py (limited to 'indra') diff --git a/indra/llcommon/tests/llleap_test.cpp b/indra/llcommon/tests/llleap_test.cpp index 0fc741d9e1..e9edd165df 100644 --- a/indra/llcommon/tests/llleap_test.cpp +++ b/indra/llcommon/tests/llleap_test.cpp @@ -17,7 +17,6 @@ // std headers #include // external library headers -#include // other Linden headers #include "../test/lltut.h" #include "../test/namedtempfile.h" @@ -29,10 +28,6 @@ #include "stringize.h" #include "StringVec.h" -using boost::assign::list_of; - -StringVec sv(const StringVec& listof) { return listof; } - #if defined(LL_WINDOWS) #define sleep(secs) _sleep((secs) * 1000) @@ -217,9 +212,9 @@ namespace tut "time.sleep(1)\n"); LLLeapVector instances; instances.push_back(LLLeap::create(get_test_name(), - sv(list_of(PYTHON)(script.getName())))->getWeak()); + StringVec{PYTHON, script.getName()})->getWeak()); instances.push_back(LLLeap::create(get_test_name(), - sv(list_of(PYTHON)(script.getName())))->getWeak()); + StringVec{PYTHON, script.getName()})->getWeak()); // In this case we're simply establishing that two LLLeap instances // can coexist without throwing exceptions or bombing in any other // way. Wait for them to terminate. @@ -249,7 +244,7 @@ namespace tut "print('Hello from Python!')\n"); CaptureLog log(LLError::LEVEL_WARN); waitfor(LLLeap::create(get_test_name(), - sv(list_of(PYTHON)(script.getName())))); + StringVec{PYTHON, script.getName()})); ensure_contains("error log line", log.messageWith("invalid protocol"), "Hello from Python!"); } @@ -264,7 +259,7 @@ namespace tut "sys.stdout.write('Hello from Python!')\n"); CaptureLog log(LLError::LEVEL_WARN); waitfor(LLLeap::create(get_test_name(), - sv(list_of(PYTHON)(script.getName())))); + StringVec{PYTHON, script.getName()})); ensure_contains("error log line", log.messageWith("Discarding"), "Hello from Python!"); } @@ -278,7 +273,7 @@ namespace tut "sys.stdout.write('5a2:something')\n"); CaptureLog log(LLError::LEVEL_WARN); waitfor(LLLeap::create(get_test_name(), - sv(list_of(PYTHON)(script.getName())))); + StringVec{PYTHON, script.getName()})); ensure_contains("error log line", log.messageWith("invalid protocol"), "5a2:"); } @@ -390,7 +385,8 @@ namespace tut "result = '' if resp == dict(pump=replypump(), data='ack')\\\n" " else 'bad: ' + str(resp)\n" "send(pump='" << result.getName() << "', data=result)\n";}); - waitfor(LLLeap::create(get_test_name(), sv(list_of(PYTHON)(script.getName())))); + waitfor(LLLeap::create(get_test_name(), + StringVec{PYTHON, script.getName()})); result.ensure(); } @@ -449,7 +445,7 @@ namespace tut " result = 'expected reqid=%s in %s' % (i, resp)\n" " break\n" "send(pump='" << result.getName() << "', data=result)\n";}); - waitfor(LLLeap::create(get_test_name(), sv(list_of(PYTHON)(script.getName()))), + waitfor(LLLeap::create(get_test_name(), StringVec{PYTHON, script.getName()}), 300); // needs more realtime than most tests result.ensure(); } @@ -516,10 +512,7 @@ namespace tut " (start, large[start:end], echoed[start:end]))\n" "sys.exit(1)\n";}); waitfor(LLLeap::create(test_name, - sv(list_of - (PYTHON) - (script.getName()) - (stringize(size)))), + StringVec{PYTHON, script.getName(), stringize(size)}), 180); // try a longer timeout result.ensure(); } diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index af99e97d66..c7d1a2c86a 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -124,17 +124,6 @@ void waitfor(LLProcess::handle h, const std::string& desc, int timeout=60) i < timeout); } -namespace { - -// find test helper, a sibling of this file -// nat 2023-07-07: we're currently using Boost 1.81, but -// path::replace_filename() (which is exactly what we need here) doesn't -// arrive until Boost 1.82. -auto test_python_script{ - (boost::filesystem::path(__FILE__).remove_filename() / "test_python_script.py").string() }; - -} - /** * Construct an LLProcess to run a Python script. */ @@ -156,7 +145,6 @@ struct PythonProcessLauncher mParams.desc = desc + " script"; mParams.executable = PYTHON; - mParams.args.add(test_python_script); mParams.args.add(mScript.getName()); } @@ -398,7 +386,6 @@ namespace tut // Have to have a named copy of this std::string so its c_str() value // will persist. std::string scriptname(script.getName()); - argv.push_back(test_python_script.c_str()); argv.push_back(scriptname.c_str()); argv.push_back(NULL); diff --git a/indra/llcommon/tests/test_python_script.py b/indra/llcommon/tests/test_python_script.py deleted file mode 100644 index c0c8661aa9..0000000000 --- a/indra/llcommon/tests/test_python_script.py +++ /dev/null @@ -1,20 +0,0 @@ -#!/usr/bin/env python3 -"""\ -@file test_python_script.py -@author Nat Goodspeed -@date 2023-07-07 -@brief Work around a problem running Python within integration tests on GitHub - Windows runners. - -$LicenseInfo:firstyear=2023&license=viewerlgpl$ -Copyright (c) 2023, Linden Research, Inc. -$/LicenseInfo$ -""" - -import os -import sys - -# use pop() so that if the referenced script in turn looks at sys.argv, it -# will see its arguments rather than its own filename -_script = sys.argv.pop(1) -exec(open(_script).read()) -- cgit v1.2.3 From 1fc8758458c99b3a41965e33b3c62613c83e403a Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 7 Jul 2023 17:31:50 -0400 Subject: SL-18837: Coax APR to log LLProcess launch attempts; show log file. --- indra/llcommon/tests/llprocess_test.cpp | 34 +++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) (limited to 'indra') diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index c7d1a2c86a..fbddc7f909 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -30,6 +30,7 @@ #include "../test/namedtempfile.h" #include "../test/catch_and_store_what_in.h" #include "stringize.h" +#include "lldir.h" #include "llsdutil.h" #include "llevents.h" #include "llstring.h" @@ -151,8 +152,37 @@ struct PythonProcessLauncher /// Launch Python script; verify that it launched void launch() { - mPy = LLProcess::create(mParams); - tut::ensure(STRINGIZE("Couldn't launch " << mDesc << " script"), bool(mPy)); + std::string logpath{ gDirUtilp->getExpandedFilename(LL_PATH_LOGS, "apr.log") }; +#if LL_WINDOWS + _putenv_s("APR_LOG", logpath.c_str()); +#else + setenv("APR_LOG", logpath.c_str(), 1); +#endif + try + { + mPy = LLProcess::create(mParams); + tut::ensure(STRINGIZE("Couldn't launch " << mDesc << " script"), bool(mPy)); + } + catch (const tut::failure& err); + { + std::ifstream inf(logpath.c_str()); + if (! inf.is_open()) + { + LL_WARNS() << "Couldn't open '" << logpath << "'" << LL_ENDL; + } + else + { + LL_WARNS() << "==============================" << LL_ENDL; + LL_WARNS() << "From '" << logpath << "':" << LL_ENDL; + std::string line; + while (std::getline(line, inf)) + { + LL_WARNS() << line << LL_ENDL; + } + LL_WARNS() << "==============================" << LL_ENDL; + } + throw; + } } /// Run Python script and wait for it to complete. -- cgit v1.2.3 From 8f81e1fa87123ff6255e9ee82e68c414efe05cdd Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 7 Jul 2023 17:47:57 -0400 Subject: SL-18837: Fix "lldir.h" #include --- indra/llcommon/tests/llprocess_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra') diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index fbddc7f909..9ee7890c7c 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -30,7 +30,7 @@ #include "../test/namedtempfile.h" #include "../test/catch_and_store_what_in.h" #include "stringize.h" -#include "lldir.h" +#include "../llfilesystem/lldir.h" #include "llsdutil.h" #include "llevents.h" #include "llstring.h" -- cgit v1.2.3 From 8aa3a0a7ed8cf3e3fedb2c98d6ea336fdd45e296 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 7 Jul 2023 19:48:02 -0400 Subject: SL-18837: Fix spurious semi --- indra/llcommon/tests/llprocess_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra') diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index 9ee7890c7c..fb5cf12cb2 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -163,7 +163,7 @@ struct PythonProcessLauncher mPy = LLProcess::create(mParams); tut::ensure(STRINGIZE("Couldn't launch " << mDesc << " script"), bool(mPy)); } - catch (const tut::failure& err); + catch (const tut::failure& err) { std::ifstream inf(logpath.c_str()); if (! inf.is_open()) -- cgit v1.2.3 From 09c5b01997e1d34e799a8a0ee3571bd181f9a665 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 7 Jul 2023 20:02:33 -0400 Subject: SL-18837: Hook in LLDir to allow reading APR log file. --- indra/llcommon/CMakeLists.txt | 2 +- indra/llcommon/tests/llprocess_test.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'indra') diff --git a/indra/llcommon/CMakeLists.txt b/indra/llcommon/CMakeLists.txt index 54020a4231..7412514e6b 100644 --- a/indra/llcommon/CMakeLists.txt +++ b/indra/llcommon/CMakeLists.txt @@ -308,7 +308,7 @@ if (LL_TESTS) LL_ADD_INTEGRATION_TEST(llleap "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llmainthreadtask "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llpounceable "" "${test_libs}") - LL_ADD_INTEGRATION_TEST(llprocess "" "${test_libs}") + LL_ADD_INTEGRATION_TEST(llprocess "" "${test_libs};llfilesystem") LL_ADD_INTEGRATION_TEST(llprocessor "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llprocinfo "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llrand "" "${test_libs}") diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index fb5cf12cb2..c6091bfeb1 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -175,7 +175,7 @@ struct PythonProcessLauncher LL_WARNS() << "==============================" << LL_ENDL; LL_WARNS() << "From '" << logpath << "':" << LL_ENDL; std::string line; - while (std::getline(line, inf)) + while (std::getline(inf, line)) { LL_WARNS() << line << LL_ENDL; } -- cgit v1.2.3 From 908fb3fed6b858da4dc2b1c840b849e30ade2046 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 7 Jul 2023 20:54:34 -0400 Subject: SL-18837: Ditch unreferenced name of caught exception --- indra/llcommon/tests/llprocess_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra') diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index c6091bfeb1..827837d62a 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -163,7 +163,7 @@ struct PythonProcessLauncher mPy = LLProcess::create(mParams); tut::ensure(STRINGIZE("Couldn't launch " << mDesc << " script"), bool(mPy)); } - catch (const tut::failure& err) + catch (const tut::failure&) { std::ifstream inf(logpath.c_str()); if (! inf.is_open()) -- cgit v1.2.3 From f37d2c307617302f2ed5dfead7e280da54a7d3e4 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Sat, 8 Jul 2023 09:04:33 -0400 Subject: SL-18837: Don't use LLDir, use NamedTempFile::temp_path. Remove llcommon circular dependency on llfilesystem, which doesn't work for this case anyway. --- indra/llcommon/CMakeLists.txt | 2 +- indra/llcommon/tests/llprocess_test.cpp | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) (limited to 'indra') diff --git a/indra/llcommon/CMakeLists.txt b/indra/llcommon/CMakeLists.txt index 7412514e6b..54020a4231 100644 --- a/indra/llcommon/CMakeLists.txt +++ b/indra/llcommon/CMakeLists.txt @@ -308,7 +308,7 @@ if (LL_TESTS) LL_ADD_INTEGRATION_TEST(llleap "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llmainthreadtask "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llpounceable "" "${test_libs}") - LL_ADD_INTEGRATION_TEST(llprocess "" "${test_libs};llfilesystem") + LL_ADD_INTEGRATION_TEST(llprocess "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llprocessor "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llprocinfo "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llrand "" "${test_libs}") diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index 827837d62a..6fcc6fd8aa 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -30,7 +30,6 @@ #include "../test/namedtempfile.h" #include "../test/catch_and_store_what_in.h" #include "stringize.h" -#include "../llfilesystem/lldir.h" #include "llsdutil.h" #include "llevents.h" #include "llstring.h" @@ -152,11 +151,9 @@ struct PythonProcessLauncher /// Launch Python script; verify that it launched void launch() { - std::string logpath{ gDirUtilp->getExpandedFilename(LL_PATH_LOGS, "apr.log") }; + std::string logpath{ NamedTempFile::temp_path("apr", ".log").string() }; #if LL_WINDOWS _putenv_s("APR_LOG", logpath.c_str()); -#else - setenv("APR_LOG", logpath.c_str(), 1); #endif try { -- cgit v1.2.3 From 1ec6c744048a2905b0f2bf83f035a8fb8798dbdf Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Sat, 8 Jul 2023 11:08:16 -0400 Subject: SL-18837: Set APR_LOG once for the whole job instead of a new value for each LLProcess::create() invocation. Since the internal apr_log() function only looks at APR_LOG once per process, the first test (which succeeded, hence no log file dump) left the log file open with that same original pathname. Resetting the APR_LOG environment variable for subsequent runs only made the new code in llprocess_test look for files that were never created. --- indra/llcommon/tests/llprocess_test.cpp | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) (limited to 'indra') diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index 6fcc6fd8aa..a01ec84547 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -151,10 +151,6 @@ struct PythonProcessLauncher /// Launch Python script; verify that it launched void launch() { - std::string logpath{ NamedTempFile::temp_path("apr", ".log").string() }; -#if LL_WINDOWS - _putenv_s("APR_LOG", logpath.c_str()); -#endif try { mPy = LLProcess::create(mParams); @@ -162,21 +158,25 @@ struct PythonProcessLauncher } catch (const tut::failure&) { - std::ifstream inf(logpath.c_str()); - if (! inf.is_open()) - { - LL_WARNS() << "Couldn't open '" << logpath << "'" << LL_ENDL; - } - else + const char* APR_LOG = getenv("APR_LOG"); + if (APR_LOG && *APR_LOG) { - LL_WARNS() << "==============================" << LL_ENDL; - LL_WARNS() << "From '" << logpath << "':" << LL_ENDL; - std::string line; - while (std::getline(inf, line)) + std::ifstream inf(APR_LOG); + if (! inf.is_open()) { - LL_WARNS() << line << LL_ENDL; + LL_WARNS() << "Couldn't open '" << APR_LOG << "'" << LL_ENDL; + } + else + { + LL_WARNS() << "==============================" << LL_ENDL; + LL_WARNS() << "From '" << APR_LOG << "':" << LL_ENDL; + std::string line; + while (std::getline(inf, line)) + { + LL_WARNS() << line << LL_ENDL; + } + LL_WARNS() << "==============================" << LL_ENDL; } - LL_WARNS() << "==============================" << LL_ENDL; } throw; } -- cgit v1.2.3 From 7dc6211ad5ea83685a35c6fff740278343aa8b9d Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Sat, 8 Jul 2023 14:08:16 -0400 Subject: SL-18837: Force llprocess_test and llleap_test to use just 'python'. On GitHub Windows runners, trying to make build.yaml set PYTHON=python in the environment doesn't work: integration tests still fail with "Access is denied" because they're still trying to execute the interpreter's full pathname. Instead, make llprocess_test and llleap_test detect the case of GitHub Windows and override the environment variable PYTHON with a baked-in string constant "python". --- indra/llcommon/tests/llleap_test.cpp | 11 ++++++++++- indra/llcommon/tests/llprocess_test.cpp | 13 ++++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) (limited to 'indra') diff --git a/indra/llcommon/tests/llleap_test.cpp b/indra/llcommon/tests/llleap_test.cpp index e9edd165df..01515ecebf 100644 --- a/indra/llcommon/tests/llleap_test.cpp +++ b/indra/llcommon/tests/llleap_test.cpp @@ -193,11 +193,20 @@ namespace tut reader.getName().substr(0, reader.getName().length()-3))), PYTHON(LLStringUtil::getenv("PYTHON")) { +#if LL_WINDOWS + // Weirdly, on GitHub Windows runners, plain 'python' works much + // better than a full pathname. + const char* RUNNER_TEMP = getenv("RUNNER_TEMP"); + if (RUNNER_TEMP && *RUNNER_TEMP) + { + PYTHON = "python"; + } +#endif ensure("Set PYTHON to interpreter pathname", !PYTHON.empty()); } NamedExtTempFile reader; const std::string reader_module; - const std::string PYTHON; + std::string PYTHON; }; typedef test_group llleap_group; typedef llleap_group::object object; diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index a01ec84547..3ba3a8aab3 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -141,6 +141,15 @@ struct PythonProcessLauncher mScript("py", script) { auto PYTHON(LLStringUtil::getenv("PYTHON")); +#if LL_WINDOWS + // Weirdly, on GitHub Windows runners, plain 'python' works much better + // than a full pathname. + const char* RUNNER_TEMP = getenv("RUNNER_TEMP"); + if (RUNNER_TEMP && *RUNNER_TEMP) + { + PYTHON = "python"; + } +#endif tut::ensure("Set $PYTHON to the Python interpreter", !PYTHON.empty()); mParams.desc = desc + " script"; @@ -1013,7 +1022,9 @@ namespace tut set_test_name("get*Pipe() validation"); PythonProcessLauncher py(get_test_name(), "from __future__ import print_function\n" - "print('this output is expected')\n"); + "import sys\n" + "print('this output is expected')\n" + "print('run by', sys.executable)\n"); py.mParams.files.add(LLProcess::FileParam("pipe")); // pipe for stdin py.mParams.files.add(LLProcess::FileParam()); // inherit stdout py.mParams.files.add(LLProcess::FileParam("pipe")); // pipe for stderr -- cgit v1.2.3 From 31ccef8a666da54312a55663a7ac03061c4903be Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 10 Jul 2023 14:35:41 -0400 Subject: SL-18837: Revert "Force llprocess_test and llleap_test to use just 'python'." Turns out that the pathname of the Python executable wasn't the issue. This reverts commit 7dc6211ad5ea83685a35c6fff740278343aa8b9d. --- indra/llcommon/tests/llleap_test.cpp | 11 +---------- indra/llcommon/tests/llprocess_test.cpp | 13 +------------ 2 files changed, 2 insertions(+), 22 deletions(-) (limited to 'indra') diff --git a/indra/llcommon/tests/llleap_test.cpp b/indra/llcommon/tests/llleap_test.cpp index 01515ecebf..e9edd165df 100644 --- a/indra/llcommon/tests/llleap_test.cpp +++ b/indra/llcommon/tests/llleap_test.cpp @@ -193,20 +193,11 @@ namespace tut reader.getName().substr(0, reader.getName().length()-3))), PYTHON(LLStringUtil::getenv("PYTHON")) { -#if LL_WINDOWS - // Weirdly, on GitHub Windows runners, plain 'python' works much - // better than a full pathname. - const char* RUNNER_TEMP = getenv("RUNNER_TEMP"); - if (RUNNER_TEMP && *RUNNER_TEMP) - { - PYTHON = "python"; - } -#endif ensure("Set PYTHON to interpreter pathname", !PYTHON.empty()); } NamedExtTempFile reader; const std::string reader_module; - std::string PYTHON; + const std::string PYTHON; }; typedef test_group llleap_group; typedef llleap_group::object object; diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index 3ba3a8aab3..a01ec84547 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -141,15 +141,6 @@ struct PythonProcessLauncher mScript("py", script) { auto PYTHON(LLStringUtil::getenv("PYTHON")); -#if LL_WINDOWS - // Weirdly, on GitHub Windows runners, plain 'python' works much better - // than a full pathname. - const char* RUNNER_TEMP = getenv("RUNNER_TEMP"); - if (RUNNER_TEMP && *RUNNER_TEMP) - { - PYTHON = "python"; - } -#endif tut::ensure("Set $PYTHON to the Python interpreter", !PYTHON.empty()); mParams.desc = desc + " script"; @@ -1022,9 +1013,7 @@ namespace tut set_test_name("get*Pipe() validation"); PythonProcessLauncher py(get_test_name(), "from __future__ import print_function\n" - "import sys\n" - "print('this output is expected')\n" - "print('run by', sys.executable)\n"); + "print('this output is expected')\n"); py.mParams.files.add(LLProcess::FileParam("pipe")); // pipe for stdin py.mParams.files.add(LLProcess::FileParam()); // inherit stdout py.mParams.files.add(LLProcess::FileParam("pipe")); // pipe for stderr -- cgit v1.2.3 From d8292a629149c2cfdda6ae9df4e87aa117153c21 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 10 Jul 2023 14:46:14 -0400 Subject: SL-18837: Disable APR_LOG for now, but leave notes for the future. --- indra/llcommon/tests/llprocess_test.cpp | 3 +++ 1 file changed, 3 insertions(+) (limited to 'indra') diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index a01ec84547..9ca664c80c 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -158,6 +158,9 @@ struct PythonProcessLauncher } catch (const tut::failure&) { + // On Windows, if APR_LOG is set, our version of APR's + // apr_create_proc() logs to the specified file. If this test + // failed, try to report that log. const char* APR_LOG = getenv("APR_LOG"); if (APR_LOG && *APR_LOG) { -- cgit v1.2.3 From c77737b925e3687e47d3a1dce1b7e8b481302741 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 10 Jul 2023 15:26:21 -0400 Subject: SL-18837: Windows failures in setWorkingDirectory(): C: vs. c: (sigh) Normalize the case of the name of the temp directory for string comparison. --- indra/llcommon/tests/llprocess_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra') diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index 9ca664c80c..c1cb2af7fe 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -591,7 +591,7 @@ namespace tut " f.write(os.path.normcase(os.path.normpath(os.getcwd())))\n"); // Before running, call setWorkingDirectory() py.mParams.cwd = tempdir.getName(); - ensure_equals("os.getcwd()", py.run_read(), tempdir.getName()); + ensure_equals("os.getcwd()", py.run_read(), utf8str_tolower(tempdir.getName())); } template<> template<> -- cgit v1.2.3 From 54f9ca5404c05a4031c1c12caf24b88048704cbd Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 17 Jul 2023 15:41:26 -0400 Subject: SL-18837: Merge branch 'actions' into actions-build-sh --- indra/llcommon/llprocessor.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra') diff --git a/indra/llcommon/llprocessor.cpp b/indra/llcommon/llprocessor.cpp index 4a1a81f083..28f8bc2b93 100644 --- a/indra/llcommon/llprocessor.cpp +++ b/indra/llcommon/llprocessor.cpp @@ -746,7 +746,7 @@ private: __cpuid(0x1, eax, ebx, ecx, edx); if(feature_infos[0] != (S32)edx) { - LL_ERRS() << "machdep.cpu.feature_bits doesn't match expected cpuid result!" << LL_ENDL; + LL_WARNS() << "machdep.cpu.feature_bits doesn't match expected cpuid result!" << LL_ENDL; } #endif // LL_RELEASE_FOR_DOWNLOAD -- cgit v1.2.3 From 167ac704c8387c531630949860e33fb5a59789a8 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 17 Jul 2023 16:20:59 -0400 Subject: SL-18837: Clean up some redundancy in llrand.cpp. --- indra/llcommon/llrand.cpp | 96 +++++++++++++++++++++++++++-------------------- 1 file changed, 55 insertions(+), 41 deletions(-) (limited to 'indra') diff --git a/indra/llcommon/llrand.cpp b/indra/llcommon/llrand.cpp index cb28a8f5c3..4e4345f37a 100644 --- a/indra/llcommon/llrand.cpp +++ b/indra/llcommon/llrand.cpp @@ -58,14 +58,18 @@ * to restore uniform distribution. */ +template +inline REAL ll_internal_random(); + // *NOTE: The system rand implementation is probably not correct. #define LL_USE_SYSTEM_RAND 0 +/***************************************************************************** +* The LL_USE_SYSTEM_RAND implementation has been disabled since June 2008. +*****************************************************************************/ #if LL_USE_SYSTEM_RAND #include -#endif -#if LL_USE_SYSTEM_RAND class LLSeedRand { public: @@ -79,25 +83,25 @@ public: } }; static LLSeedRand sRandomSeeder; -inline F64 ll_internal_random_double() + +template <> +inline F64 ll_internal_random() { #if LL_WINDOWS - return (F64)rand() / (F64)RAND_MAX; + return (F64)rand() / (F64)(RAND_MAX+1); #else return drand48(); #endif } -inline F32 ll_internal_random_float() -{ -#if LL_WINDOWS - return (F32)rand() / (F32)RAND_MAX; -#else - return (F32)drand48(); -#endif -} -#else + +/***************************************************************************** +* This is the implementation we've been using. +*****************************************************************************/ +#else // LL_USE_SYSTEM_RAND static LLRandLagFib2281 gRandomGenerator(LLUUID::getRandomSeed()); -inline F64 ll_internal_random_double() + +template <> +inline F64 ll_internal_random() { // *HACK: Through experimentation, we have found that dual core // CPUs (or at least multi-threaded processes) seem to @@ -107,16 +111,40 @@ inline F64 ll_internal_random_double() if(!((rv >= 0.0) && (rv < 1.0))) return fmod(rv, 1.0); return rv; } +#endif // LL_USE_SYSTEM_RAND + +/***************************************************************************** +* Functions common to both implementations +*****************************************************************************/ +template <> +inline F32 ll_internal_random() +{ + return F32(ll_internal_random()); +} + +/*------------------------------ F64 aliases -------------------------------*/ +inline F64 ll_internal_random_double() +{ + return ll_internal_random(); +} + +F64 ll_drand() +{ + return ll_internal_random_double(); +} +/*------------------------------ F32 aliases -------------------------------*/ inline F32 ll_internal_random_float() { - // The clamping rules are described above. - F32 rv = (F32)gRandomGenerator(); - if(!((rv >= 0.0f) && (rv < 1.0f))) return fmod(rv, 1.f); - return rv; + return ll_internal_random(); } -#endif +F32 ll_frand() +{ + return ll_internal_random_float(); +} + +/*-------------------------- clamped random range --------------------------*/ S32 ll_rand() { return ll_rand(RAND_MAX); @@ -130,42 +158,28 @@ S32 ll_rand(S32 val) return rv; } -F32 ll_frand() -{ - return ll_internal_random_float(); -} - -F32 ll_frand(F32 val) +template +REAL ll_grand(REAL val) { // The clamping rules are described above. - F32 rv = ll_internal_random_float() * val; + REAL rv = ll_internal_random() * val; if(val > 0) { - if(rv >= val) return 0.0f; + if(rv >= val) return REAL(); } else { - if(rv <= val) return 0.0f; + if(rv <= val) return REAL(); } return rv; } -F64 ll_drand() +F32 ll_frand(F32 val) { - return ll_internal_random_double(); + return ll_grand(val); } F64 ll_drand(F64 val) { - // The clamping rules are described above. - F64 rv = ll_internal_random_double() * val; - if(val > 0) - { - if(rv >= val) return 0.0; - } - else - { - if(rv <= val) return 0.0; - } - return rv; + return ll_grand(val); } -- cgit v1.2.3 From 4b158580e5654615d2a5510267bf76392c9666fa Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 17 Jul 2023 16:47:50 -0400 Subject: SL-18837: Lowercasing pathname for string compare is Windows-only. --- indra/llcommon/tests/llprocess_test.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'indra') diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index c1cb2af7fe..b6b297b8d7 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -591,7 +591,13 @@ namespace tut " f.write(os.path.normcase(os.path.normpath(os.getcwd())))\n"); // Before running, call setWorkingDirectory() py.mParams.cwd = tempdir.getName(); - ensure_equals("os.getcwd()", py.run_read(), utf8str_tolower(tempdir.getName())); + std::string expected{ tempdir.getName() }; +#if LL_WINDOWS + // SIGH, don't get tripped up by "C:" != "c:" -- + // but on the Mac, using tolower() fails because "/users" != "/Users"! + expected = utf8str_tolower(expected); +#endif + ensure_equals("os.getcwd()", py.run_read(), expected); } template<> template<> -- cgit v1.2.3 From 14d0b514af4e70956ecc2fbf6fe4c2e745e144a8 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 18 Jul 2023 09:45:00 -0400 Subject: SL-18837: Ditch inactive llrand.cpp LL_USE_SYSTEM_RAND code. LL_USE_SYSTEM_RAND has been disabled since June 2008; that code only clutters the implementation we actually use. --- indra/llcommon/llrand.cpp | 46 +++------------------------------------------- 1 file changed, 3 insertions(+), 43 deletions(-) (limited to 'indra') diff --git a/indra/llcommon/llrand.cpp b/indra/llcommon/llrand.cpp index 4e4345f37a..33afc50cf7 100644 --- a/indra/llcommon/llrand.cpp +++ b/indra/llcommon/llrand.cpp @@ -58,48 +58,12 @@ * to restore uniform distribution. */ +static LLRandLagFib2281 gRandomGenerator(LLUUID::getRandomSeed()); + +// no default implementation, only specific F64 and F32 specializations template inline REAL ll_internal_random(); -// *NOTE: The system rand implementation is probably not correct. -#define LL_USE_SYSTEM_RAND 0 - -/***************************************************************************** -* The LL_USE_SYSTEM_RAND implementation has been disabled since June 2008. -*****************************************************************************/ -#if LL_USE_SYSTEM_RAND -#include - -class LLSeedRand -{ -public: - LLSeedRand() - { -#if LL_WINDOWS - srand(LLUUID::getRandomSeed()); -#else - srand48(LLUUID::getRandomSeed()); -#endif - } -}; -static LLSeedRand sRandomSeeder; - -template <> -inline F64 ll_internal_random() -{ -#if LL_WINDOWS - return (F64)rand() / (F64)(RAND_MAX+1); -#else - return drand48(); -#endif -} - -/***************************************************************************** -* This is the implementation we've been using. -*****************************************************************************/ -#else // LL_USE_SYSTEM_RAND -static LLRandLagFib2281 gRandomGenerator(LLUUID::getRandomSeed()); - template <> inline F64 ll_internal_random() { @@ -111,11 +75,7 @@ inline F64 ll_internal_random() if(!((rv >= 0.0) && (rv < 1.0))) return fmod(rv, 1.0); return rv; } -#endif // LL_USE_SYSTEM_RAND -/***************************************************************************** -* Functions common to both implementations -*****************************************************************************/ template <> inline F32 ll_internal_random() { -- cgit v1.2.3 From ecb938c95b66ff58840aae64fd5fb791c55ec080 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 18 Jul 2023 13:38:05 -0400 Subject: SL-18837: Try waiting a couple seconds before hdiutil detach to try to avoid "Resource busy" errors from hdiutil. --- indra/newview/viewer_manifest.py | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'indra') diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index 4660991de6..1e43485b9c 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -1329,6 +1329,10 @@ class DarwinManifest(ViewerManifest): self.run_command([self.src_path_of("installers/darwin/apple-notarize.sh"), app_in_dmg]) finally: + # Empirically, on GitHub we've hit errors like: + # hdiutil: couldn't eject "disk10" - Resource busy + # Try waiting a bit to see if that improves reliability. + time.sleep(2) # Unmount the image even if exceptions from any of the above self.run_command(['hdiutil', 'detach', '-force', devfile]) -- cgit v1.2.3 From 25efba151f98308a0e2d9af52a76173be3f8aa04 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 29 Aug 2023 11:12:32 -0400 Subject: SL-18837: On Windows, LLLeap partial final line test failed. Add DEBUG log output to try to diagnose. --- indra/llcommon/llleap.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'indra') diff --git a/indra/llcommon/llleap.cpp b/indra/llcommon/llleap.cpp index c87c0758fe..060e96557d 100644 --- a/indra/llcommon/llleap.cpp +++ b/indra/llcommon/llleap.cpp @@ -376,6 +376,17 @@ public: // Read all remaining bytes and log. LL_INFOS("LLLeap") << mDesc << ": " << rest << LL_ENDL; } + /*--------------------------- diagnostic ---------------------------*/ + else if (data["eof"].asBoolean()) + { + LL_DEBUGS("LLLeap") << mDesc << " ended, no partial line" << LL_ENDL; + } + else + { + LL_DEBUGS("LLLeap") << mDesc << " (still running, " << childerr.size() + << " bytes pending)" << LL_ENDL; + } + /*------------------------- end diagnostic -------------------------*/ return false; } -- cgit v1.2.3 From e8cd5205e89993df357410c245f99ebb7703958d Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 31 Aug 2023 15:53:56 -0400 Subject: SL-19243: Try to run Windows BugSplat uploads as a separate GH job. Upload a new Windows-exe artifact containing just the executable (needed by BugSplat) separately from the artifact containing the whole NSIS installer. This requires a new viewer_exe step output set by viewer_manifest.py. Define viewer_channel and viewer_version as build job outputs. Set viewer_channel in build.yaml when tag is interpreted. Set viewer_version in build.sh at the point when it would have posted viewer_version.txt to codeticket. Add a post-windows-symbols job dependent on the build job that engages secondlife/viewer-post-bugsplat-windows, which in turn engages secondlife/post-bugsplat-windows. We keep the actual upload code in a separate repo in case we need to modify that code before rerunning to resolve upload errors. If we kept the upload code in the viewer repo itself, rerunning the upload with modifications would necessarily require rerunning the viewer build, which would defeat the purpose of SL-19243. Because of that new upload job in build.yaml, skip Windows symbol uploads in build.sh. Use a simple (platform name) artifact name for metadata because of flatten_files.py's filename collision resolution. Use hyphens, not spaces, in remaining artifact names: apparently download-artifact doesn't much like artifacts with spaces in their names. Only run the release job when in fact there's a tag. Without that, we get errors. We need not create flatten_files.py's output directory beforehand because it will do that implicitly. --- indra/newview/viewer_manifest.py | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'indra') diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index 1e43485b9c..6c4f8cb2d4 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -480,6 +480,12 @@ class WindowsManifest(ViewerManifest): if self.is_packaging_viewer(): # Find secondlife-bin.exe in the 'configuration' dir, then rename it to the result of final_exe. self.path(src='%s/secondlife-bin.exe' % self.args['configuration'], dst=self.final_exe()) + # emit that as one of the GitHub step outputs + GITHUB_OUTPUT = os.getenv('GITHUB_OUTPUT') + if GITHUB_OUTPUT: + exepath = os.path.join(self.get_dst_prefix(), self.final_exe()) + with open(GITHUB_OUTPUT, 'a') as outf: + print(f'viewer_exe={exepath}', file=outf) with self.prefix(src=os.path.join(pkgdir, "VMP")): # include the compiled launcher scripts so that it gets included in the file_list -- cgit v1.2.3 From 6a219d147d0761fce1fd75dc75f98b4e5283feb7 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 15 Aug 2023 15:32:35 -0400 Subject: SL-18837: Enlarge default coroutine stack size. A test executable on a GitHub Windows runner failed with C00000FD, which reports stack overflow. (cherry picked from commit aab7b4ba3812e5876b1205285bcfd8cff96bcac9) --- indra/llcommon/llcoros.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'indra') diff --git a/indra/llcommon/llcoros.cpp b/indra/llcommon/llcoros.cpp index 70d8dfc8b9..cfaf3415e7 100644 --- a/indra/llcommon/llcoros.cpp +++ b/indra/llcommon/llcoros.cpp @@ -123,11 +123,7 @@ LLCoros::LLCoros(): // Previously we used // boost::context::guarded_stack_allocator::default_stacksize(); // empirically this is insufficient. -#if ADDRESS_SIZE == 64 - mStackSize(512*1024), -#else - mStackSize(256*1024), -#endif + mStackSize(768*1024), // mCurrent does NOT own the current CoroData instance -- it simply // points to it. So initialize it with a no-op deleter. mCurrent{ [](CoroData*){} } -- cgit v1.2.3 From c8aa205fab621c6b88393b06268e0026eeb07a23 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 31 Aug 2023 22:35:49 -0400 Subject: SL-19243: Try to robustify GH Mac volume detach Use a retry loop very like the code-signing retry loop. --- indra/newview/viewer_manifest.py | 37 +++++++++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 8 deletions(-) (limited to 'indra') diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index 6c4f8cb2d4..aa28632e8a 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -1288,6 +1288,7 @@ class DarwinManifest(ViewerManifest): 'Keychains', 'viewer.keychain') self.run_command(['security', 'unlock-keychain', '-p', keychain_pwd, viewer_keychain]) + sign_retries=3 sign_retry_wait=15 resources = app_in_dmg + "/Contents/Resources/" plain_sign = glob.glob(resources + "llplugin/*.dylib") @@ -1296,9 +1297,10 @@ class DarwinManifest(ViewerManifest): resources + "SLPlugin.app/Contents/MacOS/SLPlugin", app_in_dmg, ] - for attempt in range(3): + for attempt in range(sign_retries): if attempt: # second or subsequent iteration - print("codesign failed, waiting {:d} seconds before retrying".format(sign_retry_wait), + print(f"codesign attempt {attempt+1} failed, " + f"waiting {sign_retry_wait:d} seconds before retrying", file=sys.stderr) time.sleep(sign_retry_wait) sign_retry_wait*=2 @@ -1329,18 +1331,37 @@ class DarwinManifest(ViewerManifest): # 'err' goes out of scope sign_failed = err else: - print("Maximum codesign attempts exceeded; giving up", file=sys.stderr) + print(f"{sign_retries} codesign attempts failed; giving up", + file=sys.stderr) raise sign_failed self.run_command(['spctl', '-a', '-texec', '-vvvv', app_in_dmg]) - self.run_command([self.src_path_of("installers/darwin/apple-notarize.sh"), app_in_dmg]) + self.run_command([self.src_path_of("installers/darwin/apple-notarize.sh"), + app_in_dmg]) finally: + # Unmount the image even if exceptions from any of the above + detach_retries = 3 + detach_retry_wait = 2 # Empirically, on GitHub we've hit errors like: # hdiutil: couldn't eject "disk10" - Resource busy - # Try waiting a bit to see if that improves reliability. - time.sleep(2) - # Unmount the image even if exceptions from any of the above - self.run_command(['hdiutil', 'detach', '-force', devfile]) + for attempt in range(detach_retries): + if attempt: # second or subsequent iteration + print(f'detach failed, waiting {detach_retry_wait} seconds before retrying', + file=sys.stderr) + # Try waiting a bit to see if that improves reliability. + time.sleep(detach_retry_wait) + detach_retry_wait *= 2 + + try: + self.run_command(['hdiutil', 'detach', '-force', devfile]) + # if no exception, the detach worked + break + except ManifestError as err: + detach_failed = err + else: + print(f'{detach_retries} detach attempts failed', file=sys.stderr) + ## can we carry on anyway?? + ## raise detach_failed print("Converting temp disk image to final disk image") self.run_command(['hdiutil', 'convert', sparsename, '-format', 'UDZO', -- cgit v1.2.3 From 00d8d4e9cdff7b5b584e7bd4f30c46902263c033 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 1 Sep 2023 12:16:36 -0400 Subject: SL-19243: Upload Mac .app as another build artifact. --- indra/newview/viewer_manifest.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) (limited to 'indra') diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index aa28632e8a..d07ec22496 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -406,6 +406,15 @@ class ViewerManifest(LLManifest): return os.path.relpath(abspath(path), abspath(base)) + def set_github_output_path(self, variable, path): + self.set_github_output(variable, os.path.join(self.get_dst_prefix(), path)) + + def set_github_output(self, variable, value): + GITHUB_OUTPUT = os.getenv('GITHUB_OUTPUT') + if GITHUB_OUTPUT: + with open(GITHUB_OUTPUT, 'a') as outf: + print('='.join((variable, value)), file=outf) + class WindowsManifest(ViewerManifest): # We want the platform, per se, for every Windows build to be 'win'. The @@ -481,11 +490,7 @@ class WindowsManifest(ViewerManifest): # Find secondlife-bin.exe in the 'configuration' dir, then rename it to the result of final_exe. self.path(src='%s/secondlife-bin.exe' % self.args['configuration'], dst=self.final_exe()) # emit that as one of the GitHub step outputs - GITHUB_OUTPUT = os.getenv('GITHUB_OUTPUT') - if GITHUB_OUTPUT: - exepath = os.path.join(self.get_dst_prefix(), self.final_exe()) - with open(GITHUB_OUTPUT, 'a') as outf: - print(f'viewer_exe={exepath}', file=outf) + self.set_github_output_path('viewer_exe', self.final_exe()) with self.prefix(src=os.path.join(pkgdir, "VMP")): # include the compiled launcher scripts so that it gets included in the file_list @@ -848,6 +853,7 @@ class DarwinManifest(ViewerManifest): def construct(self): # copy over the build result (this is a no-op if run within the xcode script) self.path(os.path.join(self.args['configuration'], self.channel()+".app"), dst="") + self.set_github_output_path('viewer_exe', self.channel() + ".app") pkgdir = os.path.join(self.args['build'], os.pardir, 'packages') relpkgdir = os.path.join(pkgdir, "lib", "release") -- cgit v1.2.3 From b13c1f77c7004ab991eb38637ccde01bc4f13b6f Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Sat, 2 Sep 2023 09:44:24 -0400 Subject: SL-19242: Emphasize to upload-artifact that our .app is a directory --- indra/newview/viewer_manifest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra') diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index d07ec22496..1f67324aaa 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -853,7 +853,7 @@ class DarwinManifest(ViewerManifest): def construct(self): # copy over the build result (this is a no-op if run within the xcode script) self.path(os.path.join(self.args['configuration'], self.channel()+".app"), dst="") - self.set_github_output_path('viewer_exe', self.channel() + ".app") + self.set_github_output_path('viewer_exe', self.channel() + ".app/") pkgdir = os.path.join(self.args['build'], os.pardir, 'packages') relpkgdir = os.path.join(pkgdir, "lib", "release") -- cgit v1.2.3 From 3eea556c28f86d1c1334879ff4d7dfc36201485e Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Sat, 2 Sep 2023 11:02:23 -0400 Subject: SL-19243: Post xcarchive.zip instead of separate symbols tarball. On Mac, in the CMake USE_BUGSPLAT logic, we created both xcarchive.zip (which is what BugSplat wants to see) and secondlife-symbols-darwin -64.tar.bz2 (which we don't think is used for anything). The tarball was posted to codeticket -- but why? If the point is to manually re-upload to BugSplat in case of failure, we'll do better saving xcarchive.zip to codeticket. For SL-19243, posting xcarchive.zip directly supports the goal of breaking out the upload to BugSplat as a separate step. Anyway, since xcarchive.zip is a superset of the tarball, the tarball can be recreated from the zip file, whereas the zip file can't be recreated from the tarball without opening the .dmg installer and extracting the viewer executable. If the xcarchive.zip file exists (that is, on Mac), post that to codeticket or GitHub, as applicable, instead of the tarball. In fact, in the USE_BUGSPLAT case, don't even bother creating the tarball since we're going to ignore it. Make the new build.sh logic that insists on BUGSPLAT_USER and BUGSPLAT_PASS conditional on BUGSPLAT_DB. --- indra/newview/CMakeLists.txt | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) (limited to 'indra') diff --git a/indra/newview/CMakeLists.txt b/indra/newview/CMakeLists.txt index dbd1f1b4ac..f23e4fa849 100644 --- a/indra/newview/CMakeLists.txt +++ b/indra/newview/CMakeLists.txt @@ -2141,20 +2141,6 @@ if (PACKAGE AND (RELEASE_CRASH_REPORTING OR NON_RELEASE_CRASH_REPORTING) AND VIE ) add_custom_target(dsym_generate DEPENDS "${VIEWER_APP_DSYM}") add_dependencies(dsym_generate ${VIEWER_BINARY_NAME}) - 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_target(dsym_tarball DEPENDS "${VIEWER_SYMBOL_FILE}") - add_dependencies(dsym_tarball dsym_generate) add_custom_command(OUTPUT "${VIEWER_APP_XCARCHIVE}" COMMAND "zip" ARGS @@ -2172,16 +2158,15 @@ if (PACKAGE AND (RELEASE_CRASH_REPORTING OR NON_RELEASE_CRASH_REPORTING) AND VIE 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}" + DEPENDS "${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" ) - add_dependencies(generate_symbols dsym_tarball dsym_xcarchive) + add_dependencies(generate_symbols dsym_xcarchive) endif (DARWIN) if (LINUX) # TBD -- cgit v1.2.3 From 2eda2eb21b720646ec8acc16d47551226f1c059d Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Sat, 2 Sep 2023 12:17:06 -0400 Subject: SL-19242: Fix duplicated 'Second Life Mumble.app' path component in the path passed as the macOS viewer_exe GitHub output. --- indra/newview/viewer_manifest.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'indra') diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index 1f67324aaa..f6282743bb 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -853,7 +853,8 @@ class DarwinManifest(ViewerManifest): def construct(self): # copy over the build result (this is a no-op if run within the xcode script) self.path(os.path.join(self.args['configuration'], self.channel()+".app"), dst="") - self.set_github_output_path('viewer_exe', self.channel() + ".app/") + # capture the entire destination app bundle + self.set_github_output_path('viewer_exe', '') pkgdir = os.path.join(self.args['build'], os.pardir, 'packages') relpkgdir = os.path.join(pkgdir, "lib", "release") -- cgit v1.2.3 From db71f834bc4b6898d8b38b484eb953924d42a5db Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 7 Sep 2023 11:44:53 -0400 Subject: SL-18837: Without USE_BUGSPLAT, no target generate_symbols. --- indra/newview/CMakeLists.txt | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'indra') diff --git a/indra/newview/CMakeLists.txt b/indra/newview/CMakeLists.txt index f23e4fa849..68f96e5669 100644 --- a/indra/newview/CMakeLists.txt +++ b/indra/newview/CMakeLists.txt @@ -2171,10 +2171,9 @@ if (PACKAGE AND (RELEASE_CRASH_REPORTING OR NON_RELEASE_CRASH_REPORTING) AND VIE if (LINUX) # TBD endif (LINUX) - endif (USE_BUGSPLAT) - # for both Bugsplat and Breakpad - add_dependencies(llpackage generate_symbols) + add_dependencies(llpackage generate_symbols) + endif (USE_BUGSPLAT) endif () if (LL_TESTS) -- cgit v1.2.3 From 95aa00f7427b7d19ab502862b3018012c1bf1904 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 7 Sep 2023 13:40:46 -0400 Subject: SL-18837: Fix minor merge glitch. --- indra/llcommon/tests/llsdserialize_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra') diff --git a/indra/llcommon/tests/llsdserialize_test.cpp b/indra/llcommon/tests/llsdserialize_test.cpp index 2de7df6f36..76e9ecc293 100644 --- a/indra/llcommon/tests/llsdserialize_test.cpp +++ b/indra/llcommon/tests/llsdserialize_test.cpp @@ -2081,7 +2081,7 @@ namespace tut " for item in DATA:\n" " serialized = llsd." << pyformatter << "(item)\n" " f.write(lenformat.pack(len(serialized)))\n" - " f.write(serialized)\n"); + " f.write(serialized)\n";}); std::ifstream inf(file.getName().c_str()); LLSD item; -- cgit v1.2.3 From a45c9f68c3e2600f48b25cc5cc74ef47cd83005b Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 8 Sep 2023 08:58:32 -0400 Subject: SL-18837: Add debugging output to llsdserialize_test.cpp. --- indra/llcommon/tests/llsdserialize_test.cpp | 90 ++++++++++++++++++++++++++++- 1 file changed, 88 insertions(+), 2 deletions(-) (limited to 'indra') diff --git a/indra/llcommon/tests/llsdserialize_test.cpp b/indra/llcommon/tests/llsdserialize_test.cpp index 76e9ecc293..ca63e74c6c 100644 --- a/indra/llcommon/tests/llsdserialize_test.cpp +++ b/indra/llcommon/tests/llsdserialize_test.cpp @@ -55,7 +55,9 @@ typedef U32 uint32_t; #include "../test/lltut.h" #include "../test/namedtempfile.h" #include "stringize.h" +#include #include +#include typedef std::function FormatterFunction; typedef std::function ParserFunction; @@ -65,6 +67,81 @@ std::vector string_to_vector(const std::string& str) return std::vector(str.begin(), str.end()); } +// Format a given byte string as 2-digit hex values, no separators +// Usage: std::cout << hexdump(somestring) << ... +class hexdump +{ +public: + hexdump(const char* data, size_t len): + hexdump(reinterpret_cast(data), len) + {} + + hexdump(const U8* data, size_t len): + mData(data, data + len) + {} + + hexdump(const hexdump&) = delete; + hexdump& operator=(const hexdump&) = delete; + + friend std::ostream& operator<<(std::ostream& out, const hexdump& self) + { + auto oldfmt{ out.flags() }; + auto oldfill{ out.fill() }; + out.setf(std::ios_base::hex, std::ios_base::basefield); + out.fill('0'); + for (auto c : self.mData) + { + out << std::setw(2) << unsigned(c); + } + out.setf(oldfmt, std::ios_base::basefield); + out.fill(oldfill); + return out; + } + +private: + std::vector mData; +}; + +// Format a given byte string as a mix of printable characters and, for each +// non-printable character, "\xnn" +// Usage: std::cout << hexmix(somestring) << ... +class hexmix +{ +public: + hexmix(const char* data, size_t len): + mData(data, len) + {} + + hexmix(const hexmix&) = delete; + hexmix& operator=(const hexmix&) = delete; + + friend std::ostream& operator<<(std::ostream& out, const hexmix& self) + { + auto oldfmt{ out.flags() }; + auto oldfill{ out.fill() }; + out.setf(std::ios_base::hex, std::ios_base::basefield); + out.fill('0'); + for (auto c : self.mData) + { + // std::isprint() must be passed an unsigned char! + if (std::isprint(static_cast(c))) + { + out << c; + } + else + { + out << "\\x" << std::setw(2) << unsigned(c); + } + } + out.setf(oldfmt, std::ios_base::basefield); + out.fill(oldfill); + return out; + } + +private: + std::string mData; +}; + namespace tut { struct sd_xml_data @@ -1909,7 +1986,14 @@ namespace tut auto buffstr{ buffer.str() }; int bufflen{ static_cast(buffstr.length()) }; out.write(reinterpret_cast(&bufflen), sizeof(bufflen)); + LL_DEBUGS("topy") << "Wrote length: " + << hexdump(reinterpret_cast(&bufflen), + sizeof(bufflen)) + << LL_ENDL; out.write(buffstr.c_str(), buffstr.length()); + LL_DEBUGS("topy") << "Wrote data: " + << hexmix(buffstr.c_str(), buffstr.length()) + << LL_ENDL; } } @@ -1938,8 +2022,8 @@ namespace tut " else:\n" " raise AssertionError('Too many data items')\n"; - // Create an llsdXXXXXX file containing 'data' serialized to - // notation. + // Create an llsdXXXXXX file containing 'data' serialized per + // FormatterFunction. NamedTempFile file("llsd", // NamedTempFile's function constructor // takes a callable. To this callable it passes the @@ -1958,11 +2042,13 @@ namespace tut "lenformat = struct.Struct('i')\n" "def parse_each(inf):\n" " for rawlen in iter(partial(inf.read, lenformat.size), b''):\n" + " print('Read length:', ''.join(('%02x' % b) for b in rawlen))\n" " len = lenformat.unpack(rawlen)[0]\n" // Since llsd.parse() has no max_bytes argument, instead of // passing the input stream directly to parse(), read the item // into a distinct bytes object and parse that. " data = inf.read(len)\n" + " print('Read data: ', repr(data))\n" " try:\n" " frombytes = llsd.parse(data)\n" " except llsd.LLSDParseError as err:\n" -- cgit v1.2.3 From eb8458587537e06df23447db56b9910a0d4e451e Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 8 Sep 2023 09:50:38 -0400 Subject: SL-18837: NamedTempFile must be binary mode on Windows. --- indra/test/namedtempfile.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'indra') diff --git a/indra/test/namedtempfile.h b/indra/test/namedtempfile.h index 525a35000d..3a994ae798 100644 --- a/indra/test/namedtempfile.h +++ b/indra/test/namedtempfile.h @@ -70,7 +70,7 @@ public: void peep() { std::cout << "File '" << mPath << "' contains:\n"; - boost::filesystem::ifstream reader(mPath); + boost::filesystem::ifstream reader(mPath, std::ios::binary); std::string line; while (std::getline(reader, line)) std::cout << line << '\n'; @@ -103,7 +103,7 @@ protected: { // Create file in a temporary place. mPath = temp_path(pfx, sfx); - boost::filesystem::ofstream out{ mPath }; + boost::filesystem::ofstream out{ mPath, std::ios::binary }; // Write desired content. func(out); } -- cgit v1.2.3 From 6cb906c44908a304af26e3ea95de88ff34ef46f7 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 8 Sep 2023 10:35:10 -0400 Subject: SL-18837: Add OpenAL::createDefaultStreamingAudioImpl(). LLAudioEngine added a new abstract virtual method that wasn't yet implemented for LLStreamingAudio_OpenAL. --- indra/llaudio/llaudioengine_openal.cpp | 5 +++++ indra/llaudio/llaudioengine_openal.h | 5 +++-- 2 files changed, 8 insertions(+), 2 deletions(-) (limited to 'indra') diff --git a/indra/llaudio/llaudioengine_openal.cpp b/indra/llaudio/llaudioengine_openal.cpp index 0a79614424..e6c62caae9 100644 --- a/indra/llaudio/llaudioengine_openal.cpp +++ b/indra/llaudio/llaudioengine_openal.cpp @@ -119,6 +119,11 @@ std::string LLAudioEngine_OpenAL::getDriverName(bool verbose) return version.str(); } +LLStreamingAudioInterface *LLAudioEngine_OpenAL::createDefaultStreamingAudioImpl() const +{ + return new LLStreamingAudio_OpenAL(); +} + // virtual void LLAudioEngine_OpenAL::allocateListener() { diff --git a/indra/llaudio/llaudioengine_openal.h b/indra/llaudio/llaudioengine_openal.h index a3cab97cd2..cd55261813 100644 --- a/indra/llaudio/llaudioengine_openal.h +++ b/indra/llaudio/llaudioengine_openal.h @@ -40,8 +40,9 @@ class LLAudioEngine_OpenAL : public LLAudioEngine LLAudioEngine_OpenAL(); virtual ~LLAudioEngine_OpenAL(); - virtual bool init(void *user_data, const std::string &app_title); - virtual std::string getDriverName(bool verbose); + virtual bool init(void *user_data, const std::string &app_title); + virtual std::string getDriverName(bool verbose); + virtual LLStreamingAudioInterface* createDefaultStreamingAudioImpl() const; virtual void allocateListener(); virtual void shutdown(); -- cgit v1.2.3 From 46bd102e80178abb094b5dac6fe9c476e044eaed Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 8 Sep 2023 11:00:16 -0400 Subject: SL-18837: Typo for LLAudioEngine_OpenAL --- indra/llaudio/llaudioengine_openal.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra') diff --git a/indra/llaudio/llaudioengine_openal.cpp b/indra/llaudio/llaudioengine_openal.cpp index e6c62caae9..d2a66c8c0f 100644 --- a/indra/llaudio/llaudioengine_openal.cpp +++ b/indra/llaudio/llaudioengine_openal.cpp @@ -121,7 +121,7 @@ std::string LLAudioEngine_OpenAL::getDriverName(bool verbose) LLStreamingAudioInterface *LLAudioEngine_OpenAL::createDefaultStreamingAudioImpl() const { - return new LLStreamingAudio_OpenAL(); + return new LLAudioEngine_OpenAL(); } // virtual -- cgit v1.2.3 From 7e655d8c828829bd45ca0ea8c47e3ce5619b23c1 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 8 Sep 2023 12:40:37 -0400 Subject: SL-18837: Revert "Typo for LLAudioEngine_OpenAL" This reverts commit 46bd102e80178abb094b5dac6fe9c476e044eaed. --- indra/llaudio/llaudioengine_openal.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra') diff --git a/indra/llaudio/llaudioengine_openal.cpp b/indra/llaudio/llaudioengine_openal.cpp index d2a66c8c0f..e6c62caae9 100644 --- a/indra/llaudio/llaudioengine_openal.cpp +++ b/indra/llaudio/llaudioengine_openal.cpp @@ -121,7 +121,7 @@ std::string LLAudioEngine_OpenAL::getDriverName(bool verbose) LLStreamingAudioInterface *LLAudioEngine_OpenAL::createDefaultStreamingAudioImpl() const { - return new LLAudioEngine_OpenAL(); + return new LLStreamingAudio_OpenAL(); } // virtual -- cgit v1.2.3 From 44181628d587f9c87cae9cdbfd57de69078000d2 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 8 Sep 2023 12:41:04 -0400 Subject: SL-18837: Revert "Add OpenAL::createDefaultStreamingAudioImpl()." This reverts commit 6cb906c44908a304af26e3ea95de88ff34ef46f7. --- indra/llaudio/llaudioengine_openal.cpp | 5 ----- indra/llaudio/llaudioengine_openal.h | 5 ++--- 2 files changed, 2 insertions(+), 8 deletions(-) (limited to 'indra') diff --git a/indra/llaudio/llaudioengine_openal.cpp b/indra/llaudio/llaudioengine_openal.cpp index e6c62caae9..0a79614424 100644 --- a/indra/llaudio/llaudioengine_openal.cpp +++ b/indra/llaudio/llaudioengine_openal.cpp @@ -119,11 +119,6 @@ std::string LLAudioEngine_OpenAL::getDriverName(bool verbose) return version.str(); } -LLStreamingAudioInterface *LLAudioEngine_OpenAL::createDefaultStreamingAudioImpl() const -{ - return new LLStreamingAudio_OpenAL(); -} - // virtual void LLAudioEngine_OpenAL::allocateListener() { diff --git a/indra/llaudio/llaudioengine_openal.h b/indra/llaudio/llaudioengine_openal.h index cd55261813..a3cab97cd2 100644 --- a/indra/llaudio/llaudioengine_openal.h +++ b/indra/llaudio/llaudioengine_openal.h @@ -40,9 +40,8 @@ class LLAudioEngine_OpenAL : public LLAudioEngine LLAudioEngine_OpenAL(); virtual ~LLAudioEngine_OpenAL(); - virtual bool init(void *user_data, const std::string &app_title); - virtual std::string getDriverName(bool verbose); - virtual LLStreamingAudioInterface* createDefaultStreamingAudioImpl() const; + virtual bool init(void *user_data, const std::string &app_title); + virtual std::string getDriverName(bool verbose); virtual void allocateListener(); virtual void shutdown(); -- cgit v1.2.3 From d459b3a1ca317c6927dffacda3aac3b580c0dfb1 Mon Sep 17 00:00:00 2001 From: Ansariel Date: Sat, 26 Aug 2023 21:04:05 +0200 Subject: Fix builds using OpenAL (cherry picked from commit fd73b6e5cf6341d606628646b73a0d05223b74bc) --- indra/llaudio/llaudioengine_openal.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra') diff --git a/indra/llaudio/llaudioengine_openal.h b/indra/llaudio/llaudioengine_openal.h index a3cab97cd2..562c96c794 100644 --- a/indra/llaudio/llaudioengine_openal.h +++ b/indra/llaudio/llaudioengine_openal.h @@ -42,6 +42,7 @@ class LLAudioEngine_OpenAL : public LLAudioEngine virtual bool init(void *user_data, const std::string &app_title); virtual std::string getDriverName(bool verbose); + virtual LLStreamingAudioInterface* createDefaultStreamingAudioImpl() const { return nullptr; } virtual void allocateListener(); virtual void shutdown(); @@ -56,7 +57,6 @@ class LLAudioEngine_OpenAL : public LLAudioEngine /*virtual*/ void updateWind(LLVector3 direction, F32 camera_altitude); private: - void * windDSP(void *newbuffer, int length); typedef S16 WIND_SAMPLE_T; LLWindGen *mWindGen; S16 *mWindBuf; -- cgit v1.2.3 From c7546ea65e55143ff3d2d82d8c289bbac7fffe0f Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 8 Sep 2023 14:14:09 -0400 Subject: SL-18837: Make llsdserialize_test debug output conditional. Move hexdump() and hexmix() stream formatters to new hexdump.h for potential use by other tests. In toPythonUsing() helper function, add a temp file to receive Python script debug output, and direct debug output to that file. On test failure, dump the contents of that file to the log. Give NamedTempFile::peep() an optional target std::ostream; refactor implementation as peep_via() that accepts a callable to process each text line. Add operator<<() to stream the contents of a NamedTempFile object to ostream -- but don't use that with LL_DEBUGS(), as it flattens the file contents into a single log line. Instead add peep_log(), which streams each individual text line to LL_DEBUGS(). --- indra/llcommon/tests/llsdserialize_test.cpp | 202 +++++++++++----------------- indra/test/hexdump.h | 97 +++++++++++++ indra/test/namedtempfile.h | 25 +++- 3 files changed, 193 insertions(+), 131 deletions(-) create mode 100644 indra/test/hexdump.h (limited to 'indra') diff --git a/indra/llcommon/tests/llsdserialize_test.cpp b/indra/llcommon/tests/llsdserialize_test.cpp index ca63e74c6c..ac40125f75 100644 --- a/indra/llcommon/tests/llsdserialize_test.cpp +++ b/indra/llcommon/tests/llsdserialize_test.cpp @@ -52,12 +52,12 @@ typedef U32 uint32_t; #include "llformat.h" #include "llmemorystream.h" +#include "../test/hexdump.h" #include "../test/lltut.h" #include "../test/namedtempfile.h" #include "stringize.h" -#include +#include "StringVec.h" #include -#include typedef std::function FormatterFunction; typedef std::function ParserFunction; @@ -67,81 +67,6 @@ std::vector string_to_vector(const std::string& str) return std::vector(str.begin(), str.end()); } -// Format a given byte string as 2-digit hex values, no separators -// Usage: std::cout << hexdump(somestring) << ... -class hexdump -{ -public: - hexdump(const char* data, size_t len): - hexdump(reinterpret_cast(data), len) - {} - - hexdump(const U8* data, size_t len): - mData(data, data + len) - {} - - hexdump(const hexdump&) = delete; - hexdump& operator=(const hexdump&) = delete; - - friend std::ostream& operator<<(std::ostream& out, const hexdump& self) - { - auto oldfmt{ out.flags() }; - auto oldfill{ out.fill() }; - out.setf(std::ios_base::hex, std::ios_base::basefield); - out.fill('0'); - for (auto c : self.mData) - { - out << std::setw(2) << unsigned(c); - } - out.setf(oldfmt, std::ios_base::basefield); - out.fill(oldfill); - return out; - } - -private: - std::vector mData; -}; - -// Format a given byte string as a mix of printable characters and, for each -// non-printable character, "\xnn" -// Usage: std::cout << hexmix(somestring) << ... -class hexmix -{ -public: - hexmix(const char* data, size_t len): - mData(data, len) - {} - - hexmix(const hexmix&) = delete; - hexmix& operator=(const hexmix&) = delete; - - friend std::ostream& operator<<(std::ostream& out, const hexmix& self) - { - auto oldfmt{ out.flags() }; - auto oldfill{ out.fill() }; - out.setf(std::ios_base::hex, std::ios_base::basefield); - out.fill('0'); - for (auto c : self.mData) - { - // std::isprint() must be passed an unsigned char! - if (std::isprint(static_cast(c))) - { - out << c; - } - else - { - out << "\\x" << std::setw(2) << unsigned(c); - } - } - out.setf(oldfmt, std::ios_base::basefield); - out.fill(oldfill); - return out; - } - -private: - std::string mData; -}; - namespace tut { struct sd_xml_data @@ -1868,16 +1793,12 @@ namespace tut // helper for TestPythonCompatible static std::string import_llsd("import os.path\n" "import sys\n" - "try:\n" - // new freestanding llsd package - " import llsd\n" - "except ImportError:\n" - // older llbase.llsd module - " from llbase import llsd\n"); + "import llsd\n"); // helper for TestPythonCompatible - template - void python(const std::string& desc, const CONTENT& script, int expect=0) + template + void python_expect(const std::string& desc, const CONTENT& script, int expect=0, + ARGS&&... args) { auto PYTHON(LLStringUtil::getenv("PYTHON")); ensure("Set $PYTHON to the Python interpreter", !PYTHON.empty()); @@ -1888,7 +1809,8 @@ namespace tut std::string q("\""); std::string qPYTHON(q + PYTHON + q); std::string qscript(q + scriptfile.getName() + q); - int rc = _spawnl(_P_WAIT, PYTHON.c_str(), qPYTHON.c_str(), qscript.c_str(), NULL); + int rc = _spawnl(_P_WAIT, PYTHON.c_str(), qPYTHON.c_str(), qscript.c_str(), + std::forward(args)..., NULL); if (rc == -1) { char buffer[256]; @@ -1904,6 +1826,10 @@ namespace tut LLProcess::Params params; params.executable = PYTHON; params.args.add(scriptfile.getName()); + for (const std::string& arg : StringVec{ std::forward(args)... }) + { + params.args.add(arg); + } LLProcessPtr py(LLProcess::create(params)); ensure(STRINGIZE("Couldn't launch " << desc << " script"), bool(py)); // Implementing timeout would mean messing with alarm() and @@ -1938,6 +1864,14 @@ namespace tut #endif } + // helper for TestPythonCompatible + template + void python(const std::string& desc, const CONTENT& script, ARGS&&... args) + { + // plain python() expects rc 0 + python_expect(desc, script, 0, std::forward(args)...); + } + struct TestPythonCompatible { TestPythonCompatible() {} @@ -1952,10 +1886,10 @@ namespace tut void TestPythonCompatibleObject::test<1>() { set_test_name("verify python()"); - python("hello", - "import sys\n" - "sys.exit(17)\n", - 17); // expect nonzero rc + python_expect("hello", + "import sys\n" + "sys.exit(17)\n", + 17); // expect nonzero rc } template<> template<> @@ -1986,14 +1920,14 @@ namespace tut auto buffstr{ buffer.str() }; int bufflen{ static_cast(buffstr.length()) }; out.write(reinterpret_cast(&bufflen), sizeof(bufflen)); - LL_DEBUGS("topy") << "Wrote length: " - << hexdump(reinterpret_cast(&bufflen), - sizeof(bufflen)) - << LL_ENDL; + LL_DEBUGS() << "Wrote length: " + << hexdump(reinterpret_cast(&bufflen), + sizeof(bufflen)) + << LL_ENDL; out.write(buffstr.c_str(), buffstr.length()); - LL_DEBUGS("topy") << "Wrote data: " - << hexmix(buffstr.c_str(), buffstr.length()) - << LL_ENDL; + LL_DEBUGS() << "Wrote data: " + << hexmix(buffstr.c_str(), buffstr.length()) + << LL_ENDL; } } @@ -2033,36 +1967,50 @@ namespace tut (std::ostream& out) { writeLLSDArray(serialize, out, cdata); }); - python("read C++ " + desc, - [&](std::ostream& out){ out << - import_llsd << - "from functools import partial\n" - "import io\n" - "import struct\n" - "lenformat = struct.Struct('i')\n" - "def parse_each(inf):\n" - " for rawlen in iter(partial(inf.read, lenformat.size), b''):\n" - " print('Read length:', ''.join(('%02x' % b) for b in rawlen))\n" - " len = lenformat.unpack(rawlen)[0]\n" - // Since llsd.parse() has no max_bytes argument, instead of - // passing the input stream directly to parse(), read the item - // into a distinct bytes object and parse that. - " data = inf.read(len)\n" - " print('Read data: ', repr(data))\n" - " try:\n" - " frombytes = llsd.parse(data)\n" - " except llsd.LLSDParseError as err:\n" - " print(f'*** {err}')\n" - " print(f'Bad content:\\n{data!r}')\n" - " raise\n" - // Also try parsing from a distinct stream. - " stream = io.BytesIO(data)\n" - " fromstream = llsd.parse(stream)\n" - " assert frombytes == fromstream\n" - " yield frombytes\n" - << pydata << - // Don't forget raw-string syntax for Windows pathnames. - "verify(parse_each(open(r'" << file.getName() << "', 'rb')))\n";}); + // 'debug' starts empty because it's intended as an output file + NamedTempFile debug("debug", ""); + + try + { + python("read C++ " + desc, + [&](std::ostream& out){ out << + import_llsd << + "from functools import partial\n" + "import io\n" + "import struct\n" + "lenformat = struct.Struct('i')\n" + "def parse_each(inf):\n" + " for rawlen in iter(partial(inf.read, lenformat.size), b''):\n" + " print('Read length:', ''.join(('%02x' % b) for b in rawlen),\n" + " file=debug)\n" + " len = lenformat.unpack(rawlen)[0]\n" + // Since llsd.parse() has no max_bytes argument, instead of + // passing the input stream directly to parse(), read the item + // into a distinct bytes object and parse that. + " data = inf.read(len)\n" + " print('Read data: ', repr(data), file=debug)\n" + " try:\n" + " frombytes = llsd.parse(data)\n" + " except llsd.LLSDParseError as err:\n" + " print(f'*** {err}')\n" + " print(f'Bad content:\\n{data!r}')\n" + " raise\n" + // Also try parsing from a distinct stream. + " stream = io.BytesIO(data)\n" + " fromstream = llsd.parse(stream)\n" + " assert frombytes == fromstream\n" + " yield frombytes\n" + << pydata << + // Don't forget raw-string syntax for Windows pathnames. + "debug = open(r'" << debug.getName() << "', 'w')\n" + "verify(parse_each(open(r'" << file.getName() << "', 'rb')))\n";}); + } + catch (const failure&) + { + LL_DEBUGS() << "Script debug output:" << LL_ENDL; + debug.peep_log(); + throw; + } } template<> template<> diff --git a/indra/test/hexdump.h b/indra/test/hexdump.h new file mode 100644 index 0000000000..dd7cbaaa3c --- /dev/null +++ b/indra/test/hexdump.h @@ -0,0 +1,97 @@ +/** + * @file hexdump.h + * @author Nat Goodspeed + * @date 2023-09-08 + * @brief Provide hexdump() and hexmix() ostream formatters + * + * $LicenseInfo:firstyear=2023&license=viewerlgpl$ + * Copyright (c) 2023, Linden Research, Inc. + * $/LicenseInfo$ + */ + +#if ! defined(LL_HEXDUMP_H) +#define LL_HEXDUMP_H + +#include +#include +#include +#include + +// Format a given byte string as 2-digit hex values, no separators +// Usage: std::cout << hexdump(somestring) << ... +class hexdump +{ +public: + hexdump(const std::string_view& data): + hexdump(data.data(), data.length()) + {} + + hexdump(const char* data, size_t len): + hexdump(reinterpret_cast(data), len) + {} + + hexdump(const unsigned char* data, size_t len): + mData(data, data + len) + {} + + friend std::ostream& operator<<(std::ostream& out, const hexdump& self) + { + auto oldfmt{ out.flags() }; + auto oldfill{ out.fill() }; + out.setf(std::ios_base::hex, std::ios_base::basefield); + out.fill('0'); + for (auto c : self.mData) + { + out << std::setw(2) << unsigned(c); + } + out.setf(oldfmt, std::ios_base::basefield); + out.fill(oldfill); + return out; + } + +private: + std::vector mData; +}; + +// Format a given byte string as a mix of printable characters and, for each +// non-printable character, "\xnn" +// Usage: std::cout << hexmix(somestring) << ... +class hexmix +{ +public: + hexmix(const std::string_view& data): + mData(data) + {} + + hexmix(const char* data, size_t len): + mData(data, len) + {} + + friend std::ostream& operator<<(std::ostream& out, const hexmix& self) + { + auto oldfmt{ out.flags() }; + auto oldfill{ out.fill() }; + out.setf(std::ios_base::hex, std::ios_base::basefield); + out.fill('0'); + for (auto c : self.mData) + { + // std::isprint() must be passed an unsigned char! + if (std::isprint(static_cast(c))) + { + out << c; + } + else + { + out << "\\x" << std::setw(2) << unsigned(c); + } + } + out.setf(oldfmt, std::ios_base::basefield); + out.fill(oldfill); + return out; + } + +private: + std::string mData; +}; + +#endif /* ! defined(LL_HEXDUMP_H) */ diff --git a/indra/test/namedtempfile.h b/indra/test/namedtempfile.h index 3a994ae798..ad14cebbd1 100644 --- a/indra/test/namedtempfile.h +++ b/indra/test/namedtempfile.h @@ -67,14 +67,31 @@ public: std::string getName() const { return mPath.string(); } - void peep() + template + void peep_via(CALLABLE&& callable) const { - std::cout << "File '" << mPath << "' contains:\n"; + std::forward(callable)(stringize("File '", mPath, "' contains:")); boost::filesystem::ifstream reader(mPath, std::ios::binary); std::string line; while (std::getline(reader, line)) - std::cout << line << '\n'; - std::cout << "---\n"; + std::forward(callable)(line); + std::forward(callable)("---"); + } + + void peep_log() const + { + peep_via([](const std::string& line){ LL_DEBUGS() << line << LL_ENDL; }); + } + + void peep(std::ostream& out=std::cout) const + { + peep_via([&out](const std::string& line){ out << line << '\n'; }); + } + + friend std::ostream& operator<<(std::ostream& out, const NamedTempFile& self) + { + self.peep(out); + return out; } static boost::filesystem::path temp_path(const std::string_view& pfx="", -- cgit v1.2.3 From 2d04cc14d3805df982d51d96d2e3d180f5ef0b34 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 8 Sep 2023 17:01:46 -0400 Subject: SL-19242: Post -app artifact, not -exe, with entire install image. Previously we posted Windows-exe, macOS-exe artifacts that were a little inconsistent: Windows-exe contained just the Windows executable, whereas macOS-exe contained the whole .app tree (but without the .app directory). Change to post Windows-app, macOS-app artifacts that each contain the whole viewer install image, including the top-level application name directory. This is what we'll need to codesign and notarize. --- indra/newview/viewer_manifest.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'indra') diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index f6282743bb..679a3441b9 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -489,8 +489,11 @@ class WindowsManifest(ViewerManifest): if self.is_packaging_viewer(): # Find secondlife-bin.exe in the 'configuration' dir, then rename it to the result of final_exe. self.path(src='%s/secondlife-bin.exe' % self.args['configuration'], dst=self.final_exe()) - # emit that as one of the GitHub step outputs - self.set_github_output_path('viewer_exe', self.final_exe()) + # Emit the whole app image as one of the GitHub step outputs. The + # current get_dst_prefix() is the top-level contents of the app + # directory -- so hop outward to the directory containing the app + # name. + self.set_github_output_path('viewer_app', os.pardir) with self.prefix(src=os.path.join(pkgdir, "VMP")): # include the compiled launcher scripts so that it gets included in the file_list @@ -853,8 +856,9 @@ class DarwinManifest(ViewerManifest): def construct(self): # copy over the build result (this is a no-op if run within the xcode script) self.path(os.path.join(self.args['configuration'], self.channel()+".app"), dst="") - # capture the entire destination app bundle - self.set_github_output_path('viewer_exe', '') + # capture the entire destination app bundle, including the containing + # .app directory + self.set_github_output_path('viewer_app', os.pardir) pkgdir = os.path.join(self.args['build'], os.pardir, 'packages') relpkgdir = os.path.join(pkgdir, "lib", "release") -- cgit v1.2.3 From ee82cd046f9bedc6a3eac96afc90f4413786669f Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 8 Sep 2023 19:25:47 -0400 Subject: SL-19242: Resolve '..' in viwer_app path before trying to upload. --- indra/newview/viewer_manifest.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'indra') diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index 679a3441b9..e93a3db6d6 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -492,8 +492,11 @@ class WindowsManifest(ViewerManifest): # Emit the whole app image as one of the GitHub step outputs. The # current get_dst_prefix() is the top-level contents of the app # directory -- so hop outward to the directory containing the app - # name. - self.set_github_output_path('viewer_app', os.pardir) + # name. But upload_artifact refuses to deal with '..', so resolve + # the path before setting viewer_app. + self.set_github_output_path( + 'viewer_app', + os.path.abspath(os.path.join(self.get_dst_prefix(), os.pardir))) with self.prefix(src=os.path.join(pkgdir, "VMP")): # include the compiled launcher scripts so that it gets included in the file_list @@ -858,7 +861,9 @@ class DarwinManifest(ViewerManifest): self.path(os.path.join(self.args['configuration'], self.channel()+".app"), dst="") # capture the entire destination app bundle, including the containing # .app directory - self.set_github_output_path('viewer_app', os.pardir) + self.set_github_output_path( + 'viewer_app', + os.path.abspath(os.path.join(self.get_dst_prefix(), os.pardir))) pkgdir = os.path.join(self.args['build'], os.pardir, 'packages') relpkgdir = os.path.join(pkgdir, "lib", "release") -- cgit v1.2.3 From dc0ebc96a4556b99b515364465b23a64e84f0899 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Sat, 9 Sep 2023 09:36:03 -0400 Subject: SL-19242: Try passing upload-artifact a normalized relative path. --- indra/newview/viewer_manifest.py | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) (limited to 'indra') diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index e93a3db6d6..dd9ca569ce 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -407,7 +407,8 @@ class ViewerManifest(LLManifest): return os.path.relpath(abspath(path), abspath(base)) def set_github_output_path(self, variable, path): - self.set_github_output(variable, os.path.join(self.get_dst_prefix(), path)) + self.set_github_output(variable, + os.path.normpath(os.path.join(self.get_dst_prefix(), path))) def set_github_output(self, variable, value): GITHUB_OUTPUT = os.getenv('GITHUB_OUTPUT') @@ -492,11 +493,8 @@ class WindowsManifest(ViewerManifest): # Emit the whole app image as one of the GitHub step outputs. The # current get_dst_prefix() is the top-level contents of the app # directory -- so hop outward to the directory containing the app - # name. But upload_artifact refuses to deal with '..', so resolve - # the path before setting viewer_app. - self.set_github_output_path( - 'viewer_app', - os.path.abspath(os.path.join(self.get_dst_prefix(), os.pardir))) + # name. + self.set_github_output_path('viewer_app', os.pardir) with self.prefix(src=os.path.join(pkgdir, "VMP")): # include the compiled launcher scripts so that it gets included in the file_list @@ -861,9 +859,7 @@ class DarwinManifest(ViewerManifest): self.path(os.path.join(self.args['configuration'], self.channel()+".app"), dst="") # capture the entire destination app bundle, including the containing # .app directory - self.set_github_output_path( - 'viewer_app', - os.path.abspath(os.path.join(self.get_dst_prefix(), os.pardir))) + self.set_github_output_path('viewer_app', os.pardir) pkgdir = os.path.join(self.args['build'], os.pardir, 'packages') relpkgdir = os.path.join(pkgdir, "lib", "release") -- cgit v1.2.3 From 29300a1fd356b7355ecfb56951e7d7ad0553ef15 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 11 Sep 2023 10:07:51 -0400 Subject: SL-19242: Try harder to post artifacts containing exactly app image. In a Windows build tree, we don't actually have an app-named top directory, so don't package its containing directory -- just the app dir itself, e.g. "newview/Release". In a Mac build tree, though we do have "Second Life Mumble.app", its parent directory also contains other large stuff. Try posting a temp directory containing a symlink to the .app. Ditch the "!*.bat" exclusion: the presence of a second path (even an exclusion) changes how upload-artifact nests its contents. --- indra/newview/viewer_manifest.py | 46 +++++++++++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 10 deletions(-) (limited to 'indra') diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index dd9ca569ce..4084c1d9a4 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -37,6 +37,7 @@ import re import shutil import subprocess import sys +import tempfile import time viewer_dir = os.path.dirname(__file__) @@ -490,11 +491,8 @@ class WindowsManifest(ViewerManifest): if self.is_packaging_viewer(): # Find secondlife-bin.exe in the 'configuration' dir, then rename it to the result of final_exe. self.path(src='%s/secondlife-bin.exe' % self.args['configuration'], dst=self.final_exe()) - # Emit the whole app image as one of the GitHub step outputs. The - # current get_dst_prefix() is the top-level contents of the app - # directory -- so hop outward to the directory containing the app - # name. - self.set_github_output_path('viewer_app', os.pardir) + # Emit the whole app image as one of the GitHub step outputs. + self.set_github_output_path('viewer_app', '') with self.prefix(src=os.path.join(pkgdir, "VMP")): # include the compiled launcher scripts so that it gets included in the file_list @@ -855,11 +853,39 @@ class DarwinManifest(ViewerManifest): return bool(set(["package", "unpacked"]).intersection(self.args['actions'])) def construct(self): - # copy over the build result (this is a no-op if run within the xcode script) - self.path(os.path.join(self.args['configuration'], self.channel()+".app"), dst="") - # capture the entire destination app bundle, including the containing - # .app directory - self.set_github_output_path('viewer_app', os.pardir) + # copy over the build result (this is a no-op if run within the xcode + # script) + appname = self.channel() + ".app" + self.path(os.path.join(self.args['configuration'], appname), dst="") + RUNNER_TEMP = os.getenv('RUNNER_TEMP') + # When running as a GitHub Action job, RUNNER_TEMP is the recommended + # temp directory. If we're not running on GitHub, don't create this + # temp directory or this symlink: we don't clean them up, trusting + # that the runner is itself transient. On a dev machine, that would + # result in temp-directory clutter. + if RUNNER_TEMP: + # We want an artifact containing the "Second Life Mumble.app" + # directory, which in turn contains the whole app bundle. + # Unfortunately, the directory that contains the .app directory + # also contains other stuff, notably the xcarchive.zip, which is + # itself enormous. Create a temp directory containing only (a link + # to) our .app dir, and specify that as the directory to upload. + wrapdir = tempfile.mkdtemp(dir=RUNNER_TEMP) + applink = os.path.join(wrapdir, appname) + # This link will be used by a different job step, so link to an + # absolute path: we can't guarantee that the other step will have + # the same current directory. + # diagnostic output + parentdir = os.path.abspath(os.path.join(self.get_dst_prefix(), os.pardir)) + for dir in parentdir, os.path.join(parentdir, appname): + print(f'Contents of {dir}:') + for item in os.listdir(dir): + print(f' {item}') + # end diagnostic output + appreal = os.path.abspath(os.path.join(self.get_dst_prefix(), os.pardir, appname)) + print(f"Linking {applink} => {appreal}") + os.symlink(appreal, applink) + self.set_github_output_path('viewer_app', wrapdir) pkgdir = os.path.join(self.args['build'], os.pardir, 'packages') relpkgdir = os.path.join(pkgdir, "lib", "release") -- cgit v1.2.3 From 0107f030c85c1ea122309a21a4d6790fc22e6d10 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 11 Sep 2023 12:53:34 -0400 Subject: SL-19242: Eliminate cruft from Windows app image artifact --- indra/newview/viewer_manifest.py | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) (limited to 'indra') diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index 4084c1d9a4..33d5720320 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -34,6 +34,7 @@ import os.path import plistlib import random import re +import secrets import shutil import subprocess import sys @@ -411,11 +412,18 @@ class ViewerManifest(LLManifest): self.set_github_output(variable, os.path.normpath(os.path.join(self.get_dst_prefix(), path))) - def set_github_output(self, variable, value): + def set_github_output(self, variable, *values): GITHUB_OUTPUT = os.getenv('GITHUB_OUTPUT') - if GITHUB_OUTPUT: + if GITHUB_OUTPUT and values: with open(GITHUB_OUTPUT, 'a') as outf: - print('='.join((variable, value)), file=outf) + if len(values) == 1: + print('='.join((variable, value)), file=outf) + else: + delim = secrets.token_hex(8) + print('<<'.join((variable, delim)), file=outf) + for value in values: + print(value, file=outf) + print(delim, file=outf) class WindowsManifest(ViewerManifest): @@ -492,7 +500,12 @@ class WindowsManifest(ViewerManifest): # Find secondlife-bin.exe in the 'configuration' dir, then rename it to the result of final_exe. self.path(src='%s/secondlife-bin.exe' % self.args['configuration'], dst=self.final_exe()) # Emit the whole app image as one of the GitHub step outputs. - self.set_github_output_path('viewer_app', '') + self.set_github_output('viewer_app', + self.get_dst_prefix(), # whole app directory + '!secondlife-bin.*', # except for this stuff + '!*.bat', + '!*.tar.bz2', + '!*.nsi') with self.prefix(src=os.path.join(pkgdir, "VMP")): # include the compiled launcher scripts so that it gets included in the file_list -- cgit v1.2.3 From 0d0dafc1be14d0c4b8cf7b02ac949182d1eb093c Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 11 Sep 2023 13:33:30 -0400 Subject: SL-19242: Fix minor error in viewer_manifest.set_github_output(). --- indra/newview/viewer_manifest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra') diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index 33d5720320..a5415bb4bc 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -417,7 +417,7 @@ class ViewerManifest(LLManifest): if GITHUB_OUTPUT and values: with open(GITHUB_OUTPUT, 'a') as outf: if len(values) == 1: - print('='.join((variable, value)), file=outf) + print('='.join((variable, values[0])), file=outf) else: delim = secrets.token_hex(8) print('<<'.join((variable, delim)), file=outf) -- cgit v1.2.3 From db2953e5de12eb5eea347f4069be9c0ab015dcdb Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 11 Sep 2023 15:29:32 -0400 Subject: SL-19242: Have to prefix upload-artifact exclude paths with pathname. --- indra/newview/viewer_manifest.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) (limited to 'indra') diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index a5415bb4bc..1a8973ff84 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -500,12 +500,15 @@ class WindowsManifest(ViewerManifest): # Find secondlife-bin.exe in the 'configuration' dir, then rename it to the result of final_exe. self.path(src='%s/secondlife-bin.exe' % self.args['configuration'], dst=self.final_exe()) # Emit the whole app image as one of the GitHub step outputs. - self.set_github_output('viewer_app', - self.get_dst_prefix(), # whole app directory - '!secondlife-bin.*', # except for this stuff - '!*.bat', - '!*.tar.bz2', - '!*.nsi') + appbase = self.relpath(self.get_dst_prefix(), base=os.getcwd()) + self.set_github_output('viewer_app', appbase, + # except for this stuff + *(('!' + os.path.join(appbase, pattern)) + for pattern in ( + 'secondlife-bin.*', + '*.bat', + '*.tar.bz2', + '*.nsi'))) with self.prefix(src=os.path.join(pkgdir, "VMP")): # include the compiled launcher scripts so that it gets included in the file_list -- cgit v1.2.3 From 62879b6e698a839784e1319e0d555c01ce0b4220 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 11 Sep 2023 18:05:32 -0400 Subject: SL-19242: Adjust Windows relative path base directory. --- indra/newview/viewer_manifest.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) (limited to 'indra') diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index 1a8973ff84..8869831635 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -499,8 +499,19 @@ class WindowsManifest(ViewerManifest): if self.is_packaging_viewer(): # Find secondlife-bin.exe in the 'configuration' dir, then rename it to the result of final_exe. self.path(src='%s/secondlife-bin.exe' % self.args['configuration'], dst=self.final_exe()) - # Emit the whole app image as one of the GitHub step outputs. - appbase = self.relpath(self.get_dst_prefix(), base=os.getcwd()) + # Emit the whole app image as one of the GitHub step outputs. When + # we feed upload-artifact multiple absolute pathnames, even just + # for exclusion, it ends up creating several extraneous directory + # levels within the artifact -- so try using only relative paths. + # One problem: as of right now, our current directory os.getcwd() + # is not the same as the initial working directory for this job + # step, meaning paths relative to our os.getcwd() won't work for + # the subsequent upload-artifact step. We're a couple directory + # levels down. Try adjusting for those when specifying the base + # for self.relpath(). + appbase = self.relpath( + self.get_dst_prefix(), + base=os.path.join(os.getcwd(), os.pardir, os.pardir)) self.set_github_output('viewer_app', appbase, # except for this stuff *(('!' + os.path.join(appbase, pattern)) -- cgit v1.2.3 From f31326189c99b18758b3754460d105a0195640c6 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 11 Sep 2023 19:38:55 -0400 Subject: SL-19242: Exclude installer from Windows-app artifact. --- indra/newview/viewer_manifest.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) (limited to 'indra') diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index 8869831635..b994b304eb 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -499,9 +499,12 @@ class WindowsManifest(ViewerManifest): if self.is_packaging_viewer(): # Find secondlife-bin.exe in the 'configuration' dir, then rename it to the result of final_exe. self.path(src='%s/secondlife-bin.exe' % self.args['configuration'], dst=self.final_exe()) - # Emit the whole app image as one of the GitHub step outputs. When - # we feed upload-artifact multiple absolute pathnames, even just - # for exclusion, it ends up creating several extraneous directory + # Emit the whole app image as one of the GitHub step outputs. We + # want the whole app -- but NOT the extraneous build products that + # get tossed into the same directory, such as the installer and + # the symbols tarball, so add exclusions. When we feed + # upload-artifact multiple absolute pathnames, even just for + # exclusion, it ends up creating several extraneous directory # levels within the artifact -- so try using only relative paths. # One problem: as of right now, our current directory os.getcwd() # is not the same as the initial working directory for this job @@ -517,6 +520,7 @@ class WindowsManifest(ViewerManifest): *(('!' + os.path.join(appbase, pattern)) for pattern in ( 'secondlife-bin.*', + '*_Setup.exe', '*.bat', '*.tar.bz2', '*.nsi'))) -- cgit v1.2.3 From be5f210e72a12eb31152c8d8c7f28c5d8930ba45 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 13 Sep 2023 11:13:55 -0400 Subject: SL-19242: Don't exclude the prepared .nsi file from Windows-app. Since we need to run NSIS in a separate job step, allow the Windows-app build artifact to include the temporary .nsi file prepared by filling in our template. Also tweak the logic that finds and runs NSIS. --- indra/newview/viewer_manifest.py | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) (limited to 'indra') diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index b994b304eb..3f07eefbec 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -522,8 +522,7 @@ class WindowsManifest(ViewerManifest): 'secondlife-bin.*', '*_Setup.exe', '*.bat', - '*.tar.bz2', - '*.nsi'))) + '*.tar.bz2'))) with self.prefix(src=os.path.join(pkgdir, "VMP")): # include the compiled launcher scripts so that it gets included in the file_list @@ -831,14 +830,20 @@ class WindowsManifest(ViewerManifest): # Check two paths, one for Program Files, and one for Program Files (x86). # Yay 64bit windows. nsis_path = "makensis.exe" - for program_files in '${programfiles}', '${programfiles(x86)}': - for nesis_path in 'NSIS', 'NSIS\\Unicode': - possible_path = os.path.expandvars(f"{program_files}\\{nesis_path}\\makensis.exe") - if os.path.exists(possible_path): - nsis_path = possible_path - break - - self.run_command([possible_path, '/V2', self.dst_path_of(tempfile)]) + try: + for program_files in os.getenv('programfiles'), os.getenv('programfiles(x86)'): + if program_files: + for subpath in 'NSIS', r'NSIS\Unicode': + possible_path = os.path.join(program_files, subpath, nsis_path) + if os.path.isfile(possible_path): + nsis_path = possible_path + # don't just break: we need to exit multiple + # levels of 'for' loop + raise StopIteration() + except StopIteration: + pass + + self.run_command([nsis_path, '/V2', self.dst_path_of(tempfile)]) self.sign(installer_file) self.created_path(self.dst_path_of(installer_file)) -- cgit v1.2.3 From b1c93e6a50cb98de154bf6cabd0d7734248c1423 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 13 Sep 2023 13:17:38 -0400 Subject: SL-19242: Add NSIS language files etc. to Windows-app artifact. --- indra/newview/viewer_manifest.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) (limited to 'indra') diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index 3f07eefbec..527aa25d05 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -805,12 +805,22 @@ class WindowsManifest(ViewerManifest): engage_registry="SetRegView 32" program_files="" + # Dump the installers/windows directory into the raw app image tree + # because NSIS needs those files. But don't use path() because we + # don't want them installed with the viewer - they're only for use by + # the installer itself. + shutil.copytree(os.path.join(self.get_src_prefix(), 'installers', 'windows'), + os.path.join(self.get_dst_prefix(), 'installers', 'windows')) + tempfile = "secondlife_setup_tmp.nsi" # the following replaces strings in the nsi template # it also does python-style % substitution self.replace_in("installers/windows/installer_template.nsi", tempfile, { "%%VERSION%%":version_vars, - "%%SOURCE%%":self.get_src_prefix(), + # The template references "%%SOURCE%%\installers\windows\...". + # Now that we've copied that directory into the app image + # tree, we can just replace %%SOURCE%% with '.'. + "%%SOURCE%%":'.', "%%INST_VARS%%":inst_vars_template % substitution_strings, "%%INSTALL_FILES%%":self.nsi_file_commands(True), "%%PROGRAMFILES%%":program_files, -- cgit v1.2.3 From baa0faa7d126e07b05d3f57d56a5440beb1c20b7 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 13 Sep 2023 15:10:22 -0400 Subject: SL-19242: Don't die if Windows app image installers/windows exists. --- indra/newview/viewer_manifest.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'indra') diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index 527aa25d05..40e6a8fa7c 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -810,7 +810,8 @@ class WindowsManifest(ViewerManifest): # don't want them installed with the viewer - they're only for use by # the installer itself. shutil.copytree(os.path.join(self.get_src_prefix(), 'installers', 'windows'), - os.path.join(self.get_dst_prefix(), 'installers', 'windows')) + os.path.join(self.get_dst_prefix(), 'installers', 'windows'), + dirs_exist_ok=True) tempfile = "secondlife_setup_tmp.nsi" # the following replaces strings in the nsi template -- cgit v1.2.3 From 62898143bff7627eeea860cd6fdf7ae865d85a60 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 13 Sep 2023 17:59:09 -0400 Subject: SL-19242: Write relative pathnames into NSIS input file. --- indra/newview/viewer_manifest.py | 53 ++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 29 deletions(-) (limited to 'indra') diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index 40e6a8fa7c..fb22d29183 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -28,6 +28,7 @@ $/LicenseInfo$ """ import errno import glob +import itertools import json import os import os.path @@ -718,46 +719,38 @@ class WindowsManifest(ViewerManifest): self.package_file = "copied_deps" def nsi_file_commands(self, install=True): - def wpath(path): - if path.endswith('/') or path.endswith(os.path.sep): - path = path[:-1] - path = path.replace('/', '\\') - return path - - result = "" + result = [] dest_files = [pair[1] for pair in self.file_list if pair[0] and os.path.isfile(pair[1])] # sort deepest hierarchy first dest_files.sort(key=lambda f: (f.count(os.path.sep), f), reverse=True) out_path = None for pkg_file in dest_files: - rel_file = os.path.normpath(pkg_file.replace(self.get_dst_prefix()+os.path.sep,'')) - installed_dir = wpath(os.path.join('$INSTDIR', os.path.dirname(rel_file))) - pkg_file = wpath(os.path.normpath(pkg_file)) - if installed_dir != out_path: - if install: - out_path = installed_dir - result += 'SetOutPath ' + out_path + '\n' + pkg_file = os.path.normpath(pkg_file) + rel_file = self.relpath(pkg_file) + installed_dir = os.path.join('$INSTDIR', os.path.dirname(rel_file)) + if install and installed_dir != out_path: + out_path = installed_dir + # emit SetOutPath every time it changes + result.append('SetOutPath ' + out_path) if install: - result += 'File ' + pkg_file + '\n' + result.append('File ' + rel_file) else: - result += 'Delete ' + wpath(os.path.join('$INSTDIR', rel_file)) + '\n' + # Note that '$INSTDIR' is purely textual here: we write + # exactly that into the .nsi file for NSIS to interpret. + result.append('Delete ' + os.path.join('$INSTDIR', rel_file)) # at the end of a delete, just rmdir all the directories if not install: - deleted_file_dirs = [os.path.dirname(pair[1].replace(self.get_dst_prefix()+os.path.sep,'')) for pair in self.file_list] - # find all ancestors so that we don't skip any dirs that happened to have no non-dir children - deleted_dirs = [] - for d in deleted_file_dirs: - deleted_dirs.extend(path_ancestors(d)) + deleted_file_dirs = [os.path.dirname(self.relpath(f)) for f in dest_files] + # find all ancestors so that we don't skip any dirs that happened + # to have no non-dir children + deleted_dirs = set(itertools.chain.from_iterable(path_ancestors(d) + for d in deleted_file_dirs)) # sort deepest hierarchy first - deleted_dirs.sort(key=lambda f: (f.count(os.path.sep), f), reverse=True) - prev = None - for d in deleted_dirs: - if d != prev: # skip duplicates - result += 'RMDir ' + wpath(os.path.join('$INSTDIR', os.path.normpath(d))) + '\n' - prev = d + for d in sorted(deleted_dirs, key=lambda f: (f.count(os.path.sep), f), reverse=True): + result.append('RMDir ' + os.path.join('$INSTDIR', os.path.normpath(d))) - return result + return '\n'.join(result) def package_finish(self): # a standard map of strings for replacing in the templates @@ -854,7 +847,9 @@ class WindowsManifest(ViewerManifest): except StopIteration: pass - self.run_command([nsis_path, '/V2', self.dst_path_of(tempfile)]) + # Because we've written relative pathnames into tempfile, run nsis + # with their base directory as current. + self.run_command([nsis_path, '/V2', tempfile], cwd=self.get_dst_prefix()) self.sign(installer_file) self.created_path(self.dst_path_of(installer_file)) -- cgit v1.2.3 From 6c6f6f402383baaa53cb14f31813654a6c6c0279 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 13 Sep 2023 19:29:45 -0400 Subject: SL-19242: Pass arbitrary subprocess kwds through run_command(). That is, make LLManifest.run_command() accept and forward subprocess keyword arguments. --- indra/lib/python/indra/util/llmanifest.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'indra') diff --git a/indra/lib/python/indra/util/llmanifest.py b/indra/lib/python/indra/util/llmanifest.py index 820f356dae..bcb9d884c3 100755 --- a/indra/lib/python/indra/util/llmanifest.py +++ b/indra/lib/python/indra/util/llmanifest.py @@ -38,6 +38,7 @@ import itertools import operator import os import re +import shlex import shutil import subprocess import sys @@ -531,15 +532,15 @@ class LLManifest(object, metaclass=LLManifestRegistry): self.cmakedirs(path) return path - def run_command(self, command): + def run_command(self, command, **kwds): """ Runs an external command. Raises ManifestError exception if the command returns a nonzero status. """ - print("Running command:", command) + print("Running command:", shlex.join(command)) sys.stdout.flush() try: - subprocess.check_call(command) + subprocess.check_call(command, **kwds) except subprocess.CalledProcessError as err: raise ManifestError( "Command %s returned non-zero status (%s)" % (command, err.returncode) ) -- cgit v1.2.3 From 2670cde77b3f75a34f537fb935464c73e9e2814a Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 13 Sep 2023 20:12:43 -0400 Subject: SL-19242: On NSIS error, dump the generated .nsi file. --- indra/newview/viewer_manifest.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'indra') diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index fb22d29183..62e64b1d44 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -849,7 +849,15 @@ class WindowsManifest(ViewerManifest): # Because we've written relative pathnames into tempfile, run nsis # with their base directory as current. - self.run_command([nsis_path, '/V2', tempfile], cwd=self.get_dst_prefix()) + try: + self.run_command([nsis_path, '/V2', tempfile], cwd=self.get_dst_prefix()) + except ManifestError as err: + print(f' {tempfile} '.center(72, '=')) + with open(self.dst_path_of(tempfile)) as nsi: + for line in nsi: + print(line) + print(72 * '=') + raise self.sign(installer_file) self.created_path(self.dst_path_of(installer_file)) -- cgit v1.2.3 From 2a031fa11cb5a5abca41d9eef5fdfb0b7bb54301 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 13 Sep 2023 21:17:34 -0400 Subject: SL-19242: Ensure NSIS file paths don't end with backslash. If they do, NSIS takes it as line continuation. --- indra/newview/viewer_manifest.py | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) (limited to 'indra') diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index 62e64b1d44..76683a1b92 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -719,6 +719,16 @@ class WindowsManifest(ViewerManifest): self.package_file = "copied_deps" def nsi_file_commands(self, install=True): + def INSTDIR(path): + # Note that '$INSTDIR' is purely textual here: we write + # exactly that into the .nsi file for NSIS to interpret. + # Pass the result through normpath() to handle the case in which + # path is the empty string. On Windows, that produces "$INSTDIR\". + # Unfortunately, if that's the last item on a line, NSIS takes + # that as line continuation and misinterprets the following line. + # Ensure we don't emit a trailing backslash. + return os.path.normpath(os.path.join('$INSTDIR', path)) + result = [] dest_files = [pair[1] for pair in self.file_list if pair[0] and os.path.isfile(pair[1])] # sort deepest hierarchy first @@ -727,7 +737,7 @@ class WindowsManifest(ViewerManifest): for pkg_file in dest_files: pkg_file = os.path.normpath(pkg_file) rel_file = self.relpath(pkg_file) - installed_dir = os.path.join('$INSTDIR', os.path.dirname(rel_file)) + installed_dir = INSTDIR(os.path.dirname(rel_file)) if install and installed_dir != out_path: out_path = installed_dir # emit SetOutPath every time it changes @@ -735,9 +745,7 @@ class WindowsManifest(ViewerManifest): if install: result.append('File ' + rel_file) else: - # Note that '$INSTDIR' is purely textual here: we write - # exactly that into the .nsi file for NSIS to interpret. - result.append('Delete ' + os.path.join('$INSTDIR', rel_file)) + result.append('Delete ' + INSTDIR(rel_file)) # at the end of a delete, just rmdir all the directories if not install: @@ -748,7 +756,7 @@ class WindowsManifest(ViewerManifest): for d in deleted_file_dirs)) # sort deepest hierarchy first for d in sorted(deleted_dirs, key=lambda f: (f.count(os.path.sep), f), reverse=True): - result.append('RMDir ' + os.path.join('$INSTDIR', os.path.normpath(d))) + result.append('RMDir ' + INSTDIR(d)) return '\n'.join(result) @@ -855,7 +863,7 @@ class WindowsManifest(ViewerManifest): print(f' {tempfile} '.center(72, '=')) with open(self.dst_path_of(tempfile)) as nsi: for line in nsi: - print(line) + print(line, end='') # already includes '\n' print(72 * '=') raise -- cgit v1.2.3 From ebe8e01ad13cf820fc89f2129928d2279e9d0f8a Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 13 Sep 2023 21:54:43 -0400 Subject: SL-19242: Capture the BugSplat @rpath as str, not bytes. --- indra/newview/viewer_manifest.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'indra') diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index 76683a1b92..0c54981af3 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -996,7 +996,8 @@ class DarwinManifest(ViewerManifest): # work, we need the build to noisily fail! oldpath = subprocess.check_output( ['objdump', '--macho', '--dylib-id', '--non-verbose', - os.path.join(relpkgdir, "BugsplatMac.framework", "BugsplatMac")] + os.path.join(relpkgdir, "BugsplatMac.framework", "BugsplatMac")], + text=True ).splitlines()[-1] # take the last line of output self.run_command( ['install_name_tool', '-change', oldpath, -- cgit v1.2.3 From e8dfbbaf880314359c0c2d18c944199e3f26db07 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 20 Sep 2023 11:34:56 -0400 Subject: SL-19242: Pass channel and imagename to sign-pkg-mac/action.yaml. The viewer_manifest.py logic to determine the name of the viewer installer .dmg is a little convoluted. Make it tell viewer-build-util/sign-pkg-mac that name, rather than passing it all the relevant inputs and composing it redundantly. sign-pkg-mac also wants the viewer channel to determine the application name. --- indra/newview/viewer_manifest.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'indra') diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index 0c54981af3..10f38fa7d8 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -1273,6 +1273,7 @@ class DarwinManifest(ViewerManifest): volname=CHANNEL_VENDOR_BASE+" Installer" # DO NOT CHANGE without understanding comment above imagename = self.installer_base_name() + self.set_github_output('imagename', imagename) sparsename = imagename + ".sparseimage" finalname = imagename + ".dmg" @@ -1292,7 +1293,7 @@ class DarwinManifest(ViewerManifest): try: devfile = re.search("/dev/disk([0-9]+)[^s]", hdi_output).group(0).strip() - volpath = re.search('HFS\s+(.+)', hdi_output).group(1).strip() + volpath = re.search(r'HFS\s+(.+)', hdi_output).group(1).strip() # Copy everything in to the mounted .dmg -- cgit v1.2.3 From ff71d3c742630751e3c7b3eeeeea334f714afd55 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 26 Sep 2023 10:22:40 -0400 Subject: SL-19242: Package Mac app image as tarball for artifact uploading. actions/upload-artifact doesn't preserve symlinks, which are important for our Mac viewer and its embedded frameworks. But tar does, so pack up the whole bundle as a tarball before posting as a GitHub artifact. --- indra/newview/viewer_manifest.py | 37 ++++++++++++++----------------------- 1 file changed, 14 insertions(+), 23 deletions(-) (limited to 'indra') diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index 10f38fa7d8..b7dc705e95 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -39,6 +39,7 @@ import secrets import shutil import subprocess import sys +import tarfile import tempfile import time @@ -918,32 +919,22 @@ class DarwinManifest(ViewerManifest): RUNNER_TEMP = os.getenv('RUNNER_TEMP') # When running as a GitHub Action job, RUNNER_TEMP is the recommended # temp directory. If we're not running on GitHub, don't create this - # temp directory or this symlink: we don't clean them up, trusting + # temp directory or this tarball: we don't clean them up, trusting # that the runner is itself transient. On a dev machine, that would # result in temp-directory clutter. if RUNNER_TEMP: - # We want an artifact containing the "Second Life Mumble.app" - # directory, which in turn contains the whole app bundle. - # Unfortunately, the directory that contains the .app directory - # also contains other stuff, notably the xcarchive.zip, which is - # itself enormous. Create a temp directory containing only (a link - # to) our .app dir, and specify that as the directory to upload. - wrapdir = tempfile.mkdtemp(dir=RUNNER_TEMP) - applink = os.path.join(wrapdir, appname) - # This link will be used by a different job step, so link to an - # absolute path: we can't guarantee that the other step will have - # the same current directory. - # diagnostic output - parentdir = os.path.abspath(os.path.join(self.get_dst_prefix(), os.pardir)) - for dir in parentdir, os.path.join(parentdir, appname): - print(f'Contents of {dir}:') - for item in os.listdir(dir): - print(f' {item}') - # end diagnostic output - appreal = os.path.abspath(os.path.join(self.get_dst_prefix(), os.pardir, appname)) - print(f"Linking {applink} => {appreal}") - os.symlink(appreal, applink) - self.set_github_output_path('viewer_app', wrapdir) + # Per GitHub's actions/upload-artifact documentation + # https://github.com/actions/upload-artifact#maintaining-file-permissions-and-case-sensitive-files + # we must package the app bundle with tar before posting as an + # artifact. Posting individual files follows symlinks, which + # causes problems, especially with frameworks: a framework's top + # level must contain symlinks into its Versions/Current, which + # must itself be a symlink to some specific Versions subdir. + tarpath = os.path.join(RUNNER_TEMP, "viewer.tar.bz2") + print(f'Creating {tarpath} from {self.get_dst_prefix()}') + with tarfile.open(tarpath, mode="x:bz2") as tarball: + tarball.add(self.get_dst_prefix()) + self.set_github_output_path('viewer_app', tarpath) pkgdir = os.path.join(self.args['build'], os.pardir, 'packages') relpkgdir = os.path.join(pkgdir, "lib", "release") -- cgit v1.2.3 From 5f69db9d1907eac79ad7fa9e639733de42eec36f Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 26 Sep 2023 11:47:18 -0400 Subject: SL-19242: Allow overwriting the Mac viewer's app bundle tarball. --- indra/newview/viewer_manifest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra') diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index b7dc705e95..899456dd09 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -932,7 +932,7 @@ class DarwinManifest(ViewerManifest): # must itself be a symlink to some specific Versions subdir. tarpath = os.path.join(RUNNER_TEMP, "viewer.tar.bz2") print(f'Creating {tarpath} from {self.get_dst_prefix()}') - with tarfile.open(tarpath, mode="x:bz2") as tarball: + with tarfile.open(tarpath, mode="w:bz2") as tarball: tarball.add(self.get_dst_prefix()) self.set_github_output_path('viewer_app', tarpath) -- cgit v1.2.3 From ae6027572816c0f67d68b8759a00cc8848e21fec Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 26 Sep 2023 13:58:19 -0400 Subject: SL-19242: Store Mac app bundle in tarball with top-level .app name. We were creating the tarball with the app bundle stored as the whole 'Users/someone/.../newview/Release/Second Life Mumble.app' path. Don't. --- indra/newview/viewer_manifest.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'indra') diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index 899456dd09..5039f3db83 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -933,7 +933,10 @@ class DarwinManifest(ViewerManifest): tarpath = os.path.join(RUNNER_TEMP, "viewer.tar.bz2") print(f'Creating {tarpath} from {self.get_dst_prefix()}') with tarfile.open(tarpath, mode="w:bz2") as tarball: - tarball.add(self.get_dst_prefix()) + # store in the tarball as just 'Second Life Mumble.app' + # instead of 'Users/someone/.../newview/Release/Second...' + tarball.add(self.get_dst_prefix(), + arcname=os.path.basename(self.get_dst_prefix())) self.set_github_output_path('viewer_app', tarpath) pkgdir = os.path.join(self.args['build'], os.pardir, 'packages') -- cgit v1.2.3 From 1684d6249984e0ad8e13263da05f84cb5e6b84fb Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 3 Oct 2023 19:02:40 -0400 Subject: SL-19242: Remove signing and packaging from viewer_manifest.py for Mac and Windows. That's now done by subsequent jobs in the GitHub build. Remove workflow step to upload installers before signing and packaging jobs. Remove from viewer_manifest.py conditionals for 32-bit Windows or Mac. Also bump to actions/checkout@v4, per dependabot. --- indra/newview/viewer_manifest.py | 323 ++------------------------------------- 1 file changed, 16 insertions(+), 307 deletions(-) (limited to 'indra') diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index 5039f3db83..7db0b75319 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -428,10 +428,11 @@ class ViewerManifest(LLManifest): print(delim, file=outf) -class WindowsManifest(ViewerManifest): +class Windows_x86_64_Manifest(ViewerManifest): # We want the platform, per se, for every Windows build to be 'win'. The # VMP will concatenate that with the address_size. build_data_json_platform = 'win' + address_size = 64 def final_exe(self): return self.exec_name()+".exe" @@ -492,7 +493,7 @@ class WindowsManifest(ViewerManifest): print("Doesn't exist:", src) def construct(self): - super(WindowsManifest, self).construct() + super().construct() pkgdir = os.path.join(self.args['build'], os.pardir, 'packages') relpkgdir = os.path.join(pkgdir, "lib", "release") @@ -575,20 +576,12 @@ class WindowsManifest(ViewerManifest): self.path("SLVoice.exe") # Vivox libraries - if (self.address_size == 64): - self.path("vivoxsdk_x64.dll") - self.path("ortp_x64.dll") - else: - self.path("vivoxsdk.dll") - self.path("ortp.dll") + self.path("vivoxsdk_x64.dll") + self.path("ortp_x64.dll") # OpenSSL - if (self.address_size == 64): - self.path("libcrypto-1_1-x64.dll") - self.path("libssl-1_1-x64.dll") - else: - self.path("libcrypto-1_1.dll") - self.path("libssl-1_1.dll") + self.path("libcrypto-1_1-x64.dll") + self.path("libssl-1_1-x64.dll") # HTTP/2 self.path("nghttp2.dll") @@ -598,14 +591,9 @@ class WindowsManifest(ViewerManifest): # BugSplat 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") + self.path("BsSndRpt64.exe") + self.path("BugSplat64.dll") + self.path("BugSplatRc64.dll") self.path(src="licenses-win32.txt", dst="licenses.txt") self.path("featuretable.txt") @@ -767,8 +755,7 @@ class WindowsManifest(ViewerManifest): 'version' : '.'.join(self.args['version']), 'version_short' : '.'.join(self.args['version'][:-1]), 'version_dashes' : '-'.join(self.args['version']), - 'version_registry' : '%s(%s)' % - ('.'.join(self.args['version']), self.address_size), + 'version_registry' : '%s(64)' % '.'.join(self.args['version']), 'final_exe' : self.final_exe(), 'flags':'', 'app_name':self.app_name(), @@ -800,12 +787,8 @@ class WindowsManifest(ViewerManifest): Caption "%(caption)s" """ - if(self.address_size == 64): - engage_registry="SetRegView 64" - program_files="!define MULTIUSER_USE_PROGRAMFILES64" - else: - engage_registry="SetRegView 32" - program_files="" + engage_registry="SetRegView 64" + program_files="!define MULTIUSER_USE_PROGRAMFILES64" # Dump the installers/windows directory into the raw app image tree # because NSIS needs those files. But don't use path() because we @@ -830,72 +813,10 @@ class WindowsManifest(ViewerManifest): "%%ENGAGEREGISTRY%%":engage_registry, "%%DELETE_FILES%%":self.nsi_file_commands(False)}) - # If we're on a build machine, sign the code using our Authenticode certificate. JC - # note that the enclosing setup exe is signed later, after the makensis makes it. - # Unlike the viewer binary, the VMP filenames are invariant with respect to version, os, etc. - for exe in ( - self.final_exe(), - "SLVersionChecker.exe", - "llplugin/dullahan_host.exe", - ): - self.sign(exe) - - # Check two paths, one for Program Files, and one for Program Files (x86). - # Yay 64bit windows. - nsis_path = "makensis.exe" - try: - for program_files in os.getenv('programfiles'), os.getenv('programfiles(x86)'): - if program_files: - for subpath in 'NSIS', r'NSIS\Unicode': - possible_path = os.path.join(program_files, subpath, nsis_path) - if os.path.isfile(possible_path): - nsis_path = possible_path - # don't just break: we need to exit multiple - # levels of 'for' loop - raise StopIteration() - except StopIteration: - pass - - # Because we've written relative pathnames into tempfile, run nsis - # with their base directory as current. - try: - self.run_command([nsis_path, '/V2', tempfile], cwd=self.get_dst_prefix()) - except ManifestError as err: - print(f' {tempfile} '.center(72, '=')) - with open(self.dst_path_of(tempfile)) as nsi: - for line in nsi: - print(line, end='') # already includes '\n' - print(72 * '=') - raise - - self.sign(installer_file) - self.created_path(self.dst_path_of(installer_file)) - self.package_file = installer_file - - def sign(self, exe): - sign_py = os.environ.get('SIGN', r'C:\buildscripts\code-signing\sign.py') - python = os.environ.get('PYTHON', sys.executable) - if os.path.exists(sign_py): - dst_path = self.dst_path_of(exe) - print("about to run signing of: ", dst_path) - self.run_command([python, sign_py, dst_path]) - else: - print("Skipping code signing of %s %s: %s not found" % (self.dst_path_of(exe), exe, sign_py)) - - def escape_slashes(self, path): - return path.replace('\\', '\\\\\\\\') -class Windows_i686_Manifest(WindowsManifest): - # Although we aren't literally passed ADDRESS_SIZE, we can infer it from - # the passed 'arch', which is used to select the specific subclass. - address_size = 32 - -class Windows_x86_64_Manifest(WindowsManifest): - address_size = 64 - - -class DarwinManifest(ViewerManifest): +class Darwin_x86_64_Manifest(ViewerManifest): build_data_json_platform = 'mac' + address_size = 64 def finish_build_data_dict(self, build_data_dict): build_data_dict.update({'Bundle Id':self.args['bundleid']}) @@ -1012,7 +933,7 @@ class DarwinManifest(ViewerManifest): with self.prefix(dst="Resources"): # defer cross-platform file copies until we're in the # nested Resources directory - super(DarwinManifest, self).construct() + super().construct() # need .icns file referenced by Info.plist with self.prefix(src=self.icon_path(), dst="") : @@ -1260,221 +1181,9 @@ class DarwinManifest(ViewerManifest): self.path( "plugins.dat" ) def package_finish(self): - global CHANNEL_VENDOR_BASE - # MBW -- If the mounted volume name changes, it breaks the .DS_Store's background image and icon positioning. - # If we really need differently named volumes, we'll need to create multiple DS_Store file images, or use some other trick. - - volname=CHANNEL_VENDOR_BASE+" Installer" # DO NOT CHANGE without understanding comment above - imagename = self.installer_base_name() self.set_github_output('imagename', imagename) - sparsename = imagename + ".sparseimage" - finalname = imagename + ".dmg" - # make sure we don't have stale files laying about - self.remove(sparsename, finalname) - - self.run_command(['hdiutil', 'create', sparsename, - '-volname', volname, '-fs', 'HFS+', - '-type', 'SPARSE', '-megabytes', '1300', - '-layout', 'SPUD']) - - # mount the image and get the name of the mount point and device node - try: - hdi_output = subprocess.check_output(['hdiutil', 'attach', '-private', sparsename], text=True) - except subprocess.CalledProcessError as err: - sys.exit("failed to mount image at '%s'" % sparsename) - - try: - devfile = re.search("/dev/disk([0-9]+)[^s]", hdi_output).group(0).strip() - volpath = re.search(r'HFS\s+(.+)', hdi_output).group(1).strip() - - # Copy everything in to the mounted .dmg - - app_name = self.app_name() - - # Hack: - # Because there is no easy way to coerce the Finder into positioning - # the app bundle in the same place with different app names, we are - # adding multiple .DS_Store files to svn. There is one for release, - # one for release candidate and one for first look. Any other channels - # will use the release .DS_Store, and will look broken. - # - Ambroff 2008-08-20 - dmg_template = os.path.join( - 'installers', 'darwin', '%s-dmg' % self.channel_type()) - - if not os.path.exists (self.src_path_of(dmg_template)): - dmg_template = os.path.join ('installers', 'darwin', 'release-dmg') - - for s,d in list({self.get_dst_prefix():app_name + ".app", - os.path.join(dmg_template, "_VolumeIcon.icns"): ".VolumeIcon.icns", - os.path.join(dmg_template, "background.jpg"): "background.jpg", - os.path.join(dmg_template, "_DS_Store"): ".DS_Store"}.items()): - print("Copying to dmg", s, d) - self.copy_action(self.src_path_of(s), os.path.join(volpath, d)) - - # Hide the background image, DS_Store file, and volume icon file (set their "visible" bit) - for f in ".VolumeIcon.icns", "background.jpg", ".DS_Store": - pathname = os.path.join(volpath, f) - self.run_command(['SetFile', '-a', 'V', pathname]) - - # Create the alias file (which is a resource file) from the .r - self.run_command( - ['Rez', self.src_path_of("installers/darwin/release-dmg/Applications-alias.r"), - '-o', os.path.join(volpath, "Applications")]) - - # Set the alias file's alias and custom icon bits - self.run_command(['SetFile', '-a', 'AC', os.path.join(volpath, "Applications")]) - - # Set the disk image root's custom icon bit - self.run_command(['SetFile', '-a', 'C', volpath]) - - # Sign the app if requested; - # do this in the copy that's in the .dmg so that the extended attributes used by - # the signature are preserved; moving the files using python will leave them behind - # and invalidate the signatures. - if 'signature' in self.args: - app_in_dmg=os.path.join(volpath,self.app_name()+".app") - print("Attempting to sign '%s'" % app_in_dmg) - identity = self.args['signature'] - if identity == '': - identity = 'Developer ID Application' - - # Look for an environment variable set via build.sh when running in Team City. - try: - build_secrets_checkout = os.environ['build_secrets_checkout'] - except KeyError: - pass - else: - # variable found so use it to unlock keychain followed by codesign - home_path = os.environ['HOME'] - keychain_pwd_path = os.path.join(build_secrets_checkout,'code-signing-osx','password.txt') - keychain_pwd = open(keychain_pwd_path).read().rstrip() - - # Note: As of macOS Sierra, keychains are created with - # names postfixed with '-db' so for example, the SL - # Viewer keychain would by default be found in - # ~/Library/Keychains/viewer.keychain-db instead of - # just ~/Library/Keychains/viewer.keychain in - # earlier versions. - # - # Because we have old OS files from previous - # versions of macOS on the build hosts, the - # configurations are different on each host. Some - # have viewer.keychain, some have viewer.keychain-db - # and some have both. As you can see in the line - # below, this script expects the Linden Developer - # cert/keys to be in viewer.keychain. - # - # To correctly sign builds you need to make sure - # ~/Library/Keychains/viewer.keychain exists on the - # host and that it contains the correct cert/key. If - # a build host is set up with a clean version of - # macOS Sierra (or later) then you will need to - # change this line (and the one for 'codesign' - # command below) to point to right place or else - # pull in the cert/key into the default viewer - # keychain 'viewer.keychain-db' and export it to - # 'viewer.keychain' - viewer_keychain = os.path.join(home_path, 'Library', - 'Keychains', 'viewer.keychain') - self.run_command(['security', 'unlock-keychain', - '-p', keychain_pwd, viewer_keychain]) - sign_retries=3 - sign_retry_wait=15 - resources = app_in_dmg + "/Contents/Resources/" - plain_sign = glob.glob(resources + "llplugin/*.dylib") - deep_sign = [ - resources + "updater/SLVersionChecker", - resources + "SLPlugin.app/Contents/MacOS/SLPlugin", - app_in_dmg, - ] - for attempt in range(sign_retries): - if attempt: # second or subsequent iteration - print(f"codesign attempt {attempt+1} failed, " - f"waiting {sign_retry_wait:d} seconds before retrying", - file=sys.stderr) - time.sleep(sign_retry_wait) - sign_retry_wait*=2 - - try: - # Note: See blurb above about names of keychains - for signee in plain_sign: - self.run_command( - ['codesign', - '--force', - '--timestamp', - '--keychain', viewer_keychain, - '--sign', identity, - signee]) - for signee in deep_sign: - self.run_command( - ['codesign', - '--verbose', - '--deep', - '--force', - '--entitlements', self.src_path_of("slplugin.entitlements"), - '--options', 'runtime', - '--keychain', viewer_keychain, - '--sign', identity, - signee]) - break # if no exception was raised, the codesign worked - except ManifestError as err: - # 'err' goes out of scope - sign_failed = err - else: - print(f"{sign_retries} codesign attempts failed; giving up", - file=sys.stderr) - raise sign_failed - self.run_command(['spctl', '-a', '-texec', '-vvvv', app_in_dmg]) - self.run_command([self.src_path_of("installers/darwin/apple-notarize.sh"), - app_in_dmg]) - - finally: - # Unmount the image even if exceptions from any of the above - detach_retries = 3 - detach_retry_wait = 2 - # Empirically, on GitHub we've hit errors like: - # hdiutil: couldn't eject "disk10" - Resource busy - for attempt in range(detach_retries): - if attempt: # second or subsequent iteration - print(f'detach failed, waiting {detach_retry_wait} seconds before retrying', - file=sys.stderr) - # Try waiting a bit to see if that improves reliability. - time.sleep(detach_retry_wait) - detach_retry_wait *= 2 - - try: - self.run_command(['hdiutil', 'detach', '-force', devfile]) - # if no exception, the detach worked - break - except ManifestError as err: - detach_failed = err - else: - print(f'{detach_retries} detach attempts failed', file=sys.stderr) - ## can we carry on anyway?? - ## raise detach_failed - - print("Converting temp disk image to final disk image") - self.run_command(['hdiutil', 'convert', sparsename, '-format', 'UDZO', - '-imagekey', 'zlib-level=9', '-o', finalname]) - # get rid of the temp file - self.package_file = finalname - self.remove(sparsename) - - -class Darwin_i386_Manifest(DarwinManifest): - address_size = 32 - - -class Darwin_i686_Manifest(DarwinManifest): - """alias in case arch is passed as i686 instead of i386""" - pass - - -class Darwin_x86_64_Manifest(DarwinManifest): - address_size = 64 - class LinuxManifest(ViewerManifest): build_data_json_platform = 'lnx' -- cgit v1.2.3 From 617801401ff84e0142fa3fe5f6614671094e2cef Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 4 Oct 2023 08:55:15 -0400 Subject: SL-18837: Restore setting ViewerManifest.package_file. This is referenced after running the packaging. --- indra/newview/viewer_manifest.py | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'indra') diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index 7db0b75319..33b2e4affc 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -813,6 +813,8 @@ class Windows_x86_64_Manifest(ViewerManifest): "%%ENGAGEREGISTRY%%":engage_registry, "%%DELETE_FILES%%":self.nsi_file_commands(False)}) + self.package_file = installer_file + class Darwin_x86_64_Manifest(ViewerManifest): build_data_json_platform = 'mac' @@ -1183,6 +1185,8 @@ class Darwin_x86_64_Manifest(ViewerManifest): def package_finish(self): imagename = self.installer_base_name() self.set_github_output('imagename', imagename) + finalname = imagename + ".dmg" + self.package_file = finalname class LinuxManifest(ViewerManifest): -- cgit v1.2.3 From 59cd3f48b00b408e354cd39c0f6e966912ba628f Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 4 Oct 2023 14:19:44 -0400 Subject: SL-18837: Fix set-but-unreferenced in LLInventoryGallery::startDrag() --- indra/newview/llinventorygallery.cpp | 2 ++ 1 file changed, 2 insertions(+) (limited to 'indra') diff --git a/indra/newview/llinventorygallery.cpp b/indra/newview/llinventorygallery.cpp index 4838ba7a47..845ea01f56 100644 --- a/indra/newview/llinventorygallery.cpp +++ b/indra/newview/llinventorygallery.cpp @@ -2420,6 +2420,8 @@ void LLInventoryGallery::startDrag() ids.push_back(selected_id); } } + // We must have set this for some reason, but it's causing compile errors + (void)src; LLToolDragAndDrop::getInstance()->beginMultiDrag(types, ids, LLToolDragAndDrop::SOURCE_AGENT); } -- cgit v1.2.3 From 7504b1c319373c950e8b8c2c7a8b2f0d9abf1d8b Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 5 Oct 2023 10:17:09 -0400 Subject: SL-18837: When llrand_test.cpp fails, display the failing value. It's frustrating and unactionable to have a failing test report merely that the random value was greater than the specified high end. Okay, so what was the value? If it's supposed to be less than the high end, did it happen to be equal? Or was it garbage? We can't reproduce the failure by rerunning! The new ensure_in_exc_range(), ensure_in_inc_range() mechanism is somewhat complex because exactly one test allows equality with the high end of the expected range, where the rest mandate that the function return less than the high end. If that's a bug in the test -- if every llrand function is supposed to return less than the high end -- then we could simplify the test logic. --- indra/llcommon/tests/llrand_test.cpp | 60 +++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 28 deletions(-) (limited to 'indra') diff --git a/indra/llcommon/tests/llrand_test.cpp b/indra/llcommon/tests/llrand_test.cpp index 383e6f9e0a..c8e2d19372 100644 --- a/indra/llcommon/tests/llrand_test.cpp +++ b/indra/llcommon/tests/llrand_test.cpp @@ -29,7 +29,30 @@ #include "../test/lltut.h" #include "../llrand.h" +#include "stringize.h" +template +void ensure_in_range_using(const std::string_view& name, + NUMBER value, NUMBER low, NUMBER high, + const std::string_view& compdesc, HIGHCOMP&& highcomp) +{ + auto failmsg{ stringize(name, " >= ", low, " (", value, ')') }; + tut::ensure(failmsg, (value >= low)); + failmsg = stringize(name, ' ', compdesc, ' ', high, " (", value, ')'); + tut::ensure(failmsg, std::forward(highcomp)(value, high)); +} + +template +void ensure_in_exc_range(const std::string_view& name, NUMBER value, NUMBER low, NUMBER high) +{ + ensure_in_range_using(name, value, low, high, "<", std::less()); +} + +template +void ensure_in_inc_range(const std::string_view& name, NUMBER value, NUMBER low, NUMBER high) +{ + ensure_in_range_using(name, value, low, high, "<=", std::less_equal()); +} namespace tut { @@ -44,84 +67,65 @@ namespace tut template<> template<> void random_object_t::test<1>() { - F32 number = 0.0f; for(S32 ii = 0; ii < 100000; ++ii) { - number = ll_frand(); - ensure("frand >= 0", (number >= 0.0f)); - ensure("frand < 1", (number < 1.0f)); + ensure_in_exc_range("frand", ll_frand(), 0.0f, 1.0f); } } template<> template<> void random_object_t::test<2>() { - F64 number = 0.0f; for(S32 ii = 0; ii < 100000; ++ii) { - number = ll_drand(); - ensure("drand >= 0", (number >= 0.0)); - ensure("drand < 1", (number < 1.0)); + ensure_in_exc_range("drand", ll_drand(), 0.0, 1.0); } } template<> template<> void random_object_t::test<3>() { - F32 number = 0.0f; for(S32 ii = 0; ii < 100000; ++ii) { - number = ll_frand(2.0f) - 1.0f; - ensure("frand >= 0", (number >= -1.0f)); - ensure("frand < 1", (number <= 1.0f)); + ensure_in_inc_range("frand(2.0f)", ll_frand(2.0f) - 1.0f, -1.0f, 1.0f); } } template<> template<> void random_object_t::test<4>() { - F32 number = 0.0f; for(S32 ii = 0; ii < 100000; ++ii) { - number = ll_frand(-7.0); - ensure("drand <= 0", (number <= 0.0)); - ensure("drand > -7", (number > -7.0)); + // Negate the result so we don't have to allow a templated low-end + // comparison as well. + ensure_in_exc_range("-frand(-7.0)", -ll_frand(-7.0), 0.0f, 7.0f); } } template<> template<> void random_object_t::test<5>() { - F64 number = 0.0f; for(S32 ii = 0; ii < 100000; ++ii) { - number = ll_drand(-2.0); - ensure("drand <= 0", (number <= 0.0)); - ensure("drand > -2", (number > -2.0)); + ensure_in_exc_range("-drand(-2.0)", -ll_drand(-2.0), 0.0, 2.0); } } template<> template<> void random_object_t::test<6>() { - S32 number = 0; for(S32 ii = 0; ii < 100000; ++ii) { - number = ll_rand(100); - ensure("rand >= 0", (number >= 0)); - ensure("rand < 100", (number < 100)); + ensure_in_exc_range("rand(100)", ll_rand(100), 0, 100); } } template<> template<> void random_object_t::test<7>() { - S32 number = 0; for(S32 ii = 0; ii < 100000; ++ii) { - number = ll_rand(-127); - ensure("rand <= 0", (number <= 0)); - ensure("rand > -127", (number > -127)); + ensure_in_exc_range("-rand(-127)", -ll_rand(-127), 0, 127); } } } -- cgit v1.2.3 From f5a34fd074bda091732a8ae0a4cf6f4e0358a140 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 12 Oct 2023 16:55:04 -0400 Subject: SL-18837: Unify all llrand_test.cpp in-range tests. The header file documents that no llrand function should ever return a value equal to the passed extent, so the one test in llrand_test.cpp that checked less than or equal to the high end of the range was anomalous. But changing that to an exclusive range means that we no longer need separate exclusive range and inclusive range functions. Replace ensure_in_range_using(), ensure_in_exc_range() and ensure_in_inc_range() with a grand unified (simplified) ensure_in_range() function. --- indra/llcommon/tests/llrand_test.cpp | 43 +++++++++++++++--------------------- 1 file changed, 18 insertions(+), 25 deletions(-) (limited to 'indra') diff --git a/indra/llcommon/tests/llrand_test.cpp b/indra/llcommon/tests/llrand_test.cpp index c8e2d19372..ac5a33d0ba 100644 --- a/indra/llcommon/tests/llrand_test.cpp +++ b/indra/llcommon/tests/llrand_test.cpp @@ -31,27 +31,20 @@ #include "../llrand.h" #include "stringize.h" -template -void ensure_in_range_using(const std::string_view& name, - NUMBER value, NUMBER low, NUMBER high, - const std::string_view& compdesc, HIGHCOMP&& highcomp) +// In llrand.h, every function is documented to return less than the high end +// -- specifically, because you can pass a negative extent, they're documented +// never to return a value equal to the extent. +// So that we don't need two different versions of ensure_in_range(), when +// testing extent < 0, negate the return value and the extent before passing +// into ensure_in_range(). +template +void ensure_in_range(const std::string_view& name, + NUMBER value, NUMBER low, NUMBER high) { auto failmsg{ stringize(name, " >= ", low, " (", value, ')') }; tut::ensure(failmsg, (value >= low)); - failmsg = stringize(name, ' ', compdesc, ' ', high, " (", value, ')'); - tut::ensure(failmsg, std::forward(highcomp)(value, high)); -} - -template -void ensure_in_exc_range(const std::string_view& name, NUMBER value, NUMBER low, NUMBER high) -{ - ensure_in_range_using(name, value, low, high, "<", std::less()); -} - -template -void ensure_in_inc_range(const std::string_view& name, NUMBER value, NUMBER low, NUMBER high) -{ - ensure_in_range_using(name, value, low, high, "<=", std::less_equal()); + failmsg = stringize(name, " < ", high, " (", value, ')'); + tut::ensure(failmsg, (value < high)); } namespace tut @@ -69,7 +62,7 @@ namespace tut { for(S32 ii = 0; ii < 100000; ++ii) { - ensure_in_exc_range("frand", ll_frand(), 0.0f, 1.0f); + ensure_in_range("frand", ll_frand(), 0.0f, 1.0f); } } @@ -78,7 +71,7 @@ namespace tut { for(S32 ii = 0; ii < 100000; ++ii) { - ensure_in_exc_range("drand", ll_drand(), 0.0, 1.0); + ensure_in_range("drand", ll_drand(), 0.0, 1.0); } } @@ -87,7 +80,7 @@ namespace tut { for(S32 ii = 0; ii < 100000; ++ii) { - ensure_in_inc_range("frand(2.0f)", ll_frand(2.0f) - 1.0f, -1.0f, 1.0f); + ensure_in_range("frand(2.0f)", ll_frand(2.0f) - 1.0f, -1.0f, 1.0f); } } @@ -98,7 +91,7 @@ namespace tut { // Negate the result so we don't have to allow a templated low-end // comparison as well. - ensure_in_exc_range("-frand(-7.0)", -ll_frand(-7.0), 0.0f, 7.0f); + ensure_in_range("-frand(-7.0)", -ll_frand(-7.0), 0.0f, 7.0f); } } @@ -107,7 +100,7 @@ namespace tut { for(S32 ii = 0; ii < 100000; ++ii) { - ensure_in_exc_range("-drand(-2.0)", -ll_drand(-2.0), 0.0, 2.0); + ensure_in_range("-drand(-2.0)", -ll_drand(-2.0), 0.0, 2.0); } } @@ -116,7 +109,7 @@ namespace tut { for(S32 ii = 0; ii < 100000; ++ii) { - ensure_in_exc_range("rand(100)", ll_rand(100), 0, 100); + ensure_in_range("rand(100)", ll_rand(100), 0, 100); } } @@ -125,7 +118,7 @@ namespace tut { for(S32 ii = 0; ii < 100000; ++ii) { - ensure_in_exc_range("-rand(-127)", -ll_rand(-127), 0, 127); + ensure_in_range("-rand(-127)", -ll_rand(-127), 0, 127); } } } -- cgit v1.2.3 From 016023c780aeb02b281ed52128f286b08d3c9eaf Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 16 Oct 2023 19:04:23 -0400 Subject: SL-18837: Create Second Life Viewer.app, not Second Life Release.app --- indra/newview/viewer_manifest.py | 48 ++++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 24 deletions(-) (limited to 'indra') diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index 33b2e4affc..4bcc26e1d5 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -837,30 +837,7 @@ class Darwin_x86_64_Manifest(ViewerManifest): def construct(self): # copy over the build result (this is a no-op if run within the xcode # script) - appname = self.channel() + ".app" - self.path(os.path.join(self.args['configuration'], appname), dst="") - RUNNER_TEMP = os.getenv('RUNNER_TEMP') - # When running as a GitHub Action job, RUNNER_TEMP is the recommended - # temp directory. If we're not running on GitHub, don't create this - # temp directory or this tarball: we don't clean them up, trusting - # that the runner is itself transient. On a dev machine, that would - # result in temp-directory clutter. - if RUNNER_TEMP: - # Per GitHub's actions/upload-artifact documentation - # https://github.com/actions/upload-artifact#maintaining-file-permissions-and-case-sensitive-files - # we must package the app bundle with tar before posting as an - # artifact. Posting individual files follows symlinks, which - # causes problems, especially with frameworks: a framework's top - # level must contain symlinks into its Versions/Current, which - # must itself be a symlink to some specific Versions subdir. - tarpath = os.path.join(RUNNER_TEMP, "viewer.tar.bz2") - print(f'Creating {tarpath} from {self.get_dst_prefix()}') - with tarfile.open(tarpath, mode="w:bz2") as tarball: - # store in the tarball as just 'Second Life Mumble.app' - # instead of 'Users/someone/.../newview/Release/Second...' - tarball.add(self.get_dst_prefix(), - arcname=os.path.basename(self.get_dst_prefix())) - self.set_github_output_path('viewer_app', tarpath) + self.path(os.path.join(self.args['configuration'], self.app_name() + ".app"), dst="") pkgdir = os.path.join(self.args['build'], os.pardir, 'packages') relpkgdir = os.path.join(pkgdir, "lib", "release") @@ -1188,6 +1165,29 @@ class Darwin_x86_64_Manifest(ViewerManifest): finalname = imagename + ".dmg" self.package_file = finalname + RUNNER_TEMP = os.getenv('RUNNER_TEMP') + # When running as a GitHub Action job, RUNNER_TEMP is the recommended + # temp directory. If we're not running on GitHub, don't create this + # temp directory or this tarball: we don't clean them up, trusting + # that the runner is itself transient. On a dev machine, that would + # result in temp-directory clutter. + if RUNNER_TEMP: + # Per GitHub's actions/upload-artifact documentation + # https://github.com/actions/upload-artifact#maintaining-file-permissions-and-case-sensitive-files + # we must package the app bundle with tar before posting as an + # artifact. Posting individual files follows symlinks, which + # causes problems, especially with frameworks: a framework's top + # level must contain symlinks into its Versions/Current, which + # must itself be a symlink to some specific Versions subdir. + tarpath = os.path.join(RUNNER_TEMP, "viewer.tar.bz2") + print(f'Creating {tarpath} from {self.get_dst_prefix()}') + with tarfile.open(tarpath, mode="w:bz2") as tarball: + # store in the tarball as just 'Second Life Mumble.app' + # instead of 'Users/someone/.../newview/Release/Second...' + tarball.add(self.get_dst_prefix(), + arcname=os.path.basename(self.get_dst_prefix())) + self.set_github_output_path('viewer_app', tarpath) + class LinuxManifest(ViewerManifest): build_data_json_platform = 'lnx' -- cgit v1.2.3 From 19f453fc2007f780ae5d819090db206f07d0a9c6 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 16 Oct 2023 19:46:37 -0400 Subject: SL-18837: Second Life Release.app=>Second Life Viewer.app in tarball --- indra/newview/viewer_manifest.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'indra') diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index 4bcc26e1d5..1fa4df1682 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -837,7 +837,7 @@ class Darwin_x86_64_Manifest(ViewerManifest): def construct(self): # copy over the build result (this is a no-op if run within the xcode # script) - self.path(os.path.join(self.args['configuration'], self.app_name() + ".app"), dst="") + self.path(os.path.join(self.args['configuration'], self.channel() + ".app"), dst="") pkgdir = os.path.join(self.args['build'], os.pardir, 'packages') relpkgdir = os.path.join(pkgdir, "lib", "release") @@ -1182,10 +1182,12 @@ class Darwin_x86_64_Manifest(ViewerManifest): tarpath = os.path.join(RUNNER_TEMP, "viewer.tar.bz2") print(f'Creating {tarpath} from {self.get_dst_prefix()}') with tarfile.open(tarpath, mode="w:bz2") as tarball: - # store in the tarball as just 'Second Life Mumble.app' + # Store in the tarball as just 'Second Life Mumble.app' # instead of 'Users/someone/.../newview/Release/Second...' + # It's at this point that we rename 'Second Life Release.app' + # to 'Second Life Viewer.app'. tarball.add(self.get_dst_prefix(), - arcname=os.path.basename(self.get_dst_prefix())) + arcname=self.app_name() + ".app") self.set_github_output_path('viewer_app', tarpath) -- cgit v1.2.3 From 117f07e5a4b7882a44681c730dcc0628238cfec6 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 17 Oct 2023 06:33:05 -0400 Subject: SL-18837: Avoid stuffing build number into 32-bit int. Even though LLVersionInfo::getBuild() already returns a 64-bit int, various consumers assumed it could fit into 32 bits. It was especially bad to pass it to a classic C style varargs function. Only on a little-endian CPU, and only because it was the last argument, the damage was limited to truncation -- instead of arbitrary undefined behavior. Where the consumer doesn't support 64-bit ints, pass as string instead. --- indra/newview/llappviewer.cpp | 4 +++- indra/newview/llcurrencyuimanager.cpp | 13 +++++++++---- indra/newview/llpanellogin.cpp | 13 ++++++------- indra/newview/lltranslate.cpp | 25 +++++++++++++------------ indra/newview/llversioninfo.cpp | 4 ++-- indra/newview/llversioninfo.h | 2 +- indra/newview/llxmlrpctransaction.cpp | 15 ++++++++------- 7 files changed, 42 insertions(+), 34 deletions(-) (limited to 'indra') diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index dec849afa1..5763ebe721 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -3209,8 +3209,10 @@ LLSD LLAppViewer::getViewerInfo() const // LLFloaterAbout. LLSD info; auto& versionInfo(LLVersionInfo::instance()); + // With GitHub builds, the build number is too big to fit in a 32-bit int, + // and LLSD doesn't deal with integers wider than int. Use string. info["VIEWER_VERSION"] = llsd::array(versionInfo.getMajor(), versionInfo.getMinor(), - versionInfo.getPatch(), versionInfo.getBuild()); + versionInfo.getPatch(), stringize(versionInfo.getBuild())); info["VIEWER_VERSION_STR"] = versionInfo.getVersion(); info["CHANNEL"] = versionInfo.getChannel(); info["ADDRESS_SIZE"] = ADDRESS_SIZE; diff --git a/indra/newview/llcurrencyuimanager.cpp b/indra/newview/llcurrencyuimanager.cpp index 232e461fd0..4c0a5cf183 100644 --- a/indra/newview/llcurrencyuimanager.cpp +++ b/indra/newview/llcurrencyuimanager.cpp @@ -45,6 +45,7 @@ #include "llxmlrpctransaction.h" #include "llviewernetwork.h" #include "llpanel.h" +#include "stringize.h" const F64 CURRENCY_ESTIMATE_FREQUENCY = 2.0; @@ -158,7 +159,7 @@ void LLCurrencyUIManager::Impl::updateCurrencyInfo() mLocalCurrencyEstimated = true; return; } - + LLXMLRPCValue keywordArgs = LLXMLRPCValue::createStruct(); keywordArgs.appendString("agentId", gAgent.getID().asString()); keywordArgs.appendString( @@ -170,8 +171,10 @@ void LLCurrencyUIManager::Impl::updateCurrencyInfo() keywordArgs.appendInt("viewerMajorVersion", LLVersionInfo::instance().getMajor()); keywordArgs.appendInt("viewerMinorVersion", LLVersionInfo::instance().getMinor()); keywordArgs.appendInt("viewerPatchVersion", LLVersionInfo::instance().getPatch()); - keywordArgs.appendInt("viewerBuildVersion", LLVersionInfo::instance().getBuild()); - + // With GitHub builds, the build number is too big to fit in a 32-bit int, + // and XMLRPC_VALUE doesn't deal with integers wider than int. Use string. + keywordArgs.appendString("viewerBuildVersion", stringize(LLVersionInfo::instance().getBuild())); + LLXMLRPCValue params = LLXMLRPCValue::createArray(); params.append(keywordArgs); @@ -245,7 +248,9 @@ void LLCurrencyUIManager::Impl::startCurrencyBuy(const std::string& password) keywordArgs.appendInt("viewerMajorVersion", LLVersionInfo::instance().getMajor()); keywordArgs.appendInt("viewerMinorVersion", LLVersionInfo::instance().getMinor()); keywordArgs.appendInt("viewerPatchVersion", LLVersionInfo::instance().getPatch()); - keywordArgs.appendInt("viewerBuildVersion", LLVersionInfo::instance().getBuild()); + // With GitHub builds, the build number is too big to fit in a 32-bit int, + // and XMLRPC_VALUE doesn't deal with integers wider than int. Use string. + keywordArgs.appendString("viewerBuildVersion", stringize(LLVersionInfo::instance().getBuild())); LLXMLRPCValue params = LLXMLRPCValue::createArray(); params.append(keywordArgs); diff --git a/indra/newview/llpanellogin.cpp b/indra/newview/llpanellogin.cpp index ea936ab024..025a653c47 100644 --- a/indra/newview/llpanellogin.cpp +++ b/indra/newview/llpanellogin.cpp @@ -65,6 +65,7 @@ #include "lltrans.h" #include "llglheaders.h" #include "llpanelloginlistener.h" +#include "stringize.h" #if LL_WINDOWS #pragma warning(disable: 4355) // 'this' used in initializer list @@ -300,10 +301,9 @@ LLPanelLogin::LLPanelLogin(const LLRect &rect, setDefaultBtn(def_btn); std::string channel = LLVersionInfo::instance().getChannel(); - std::string version = llformat("%s (%ld)", - LLVersionInfo::instance().getShortVersion().c_str(), - LLVersionInfo::instance().getBuild()); - + std::string version = stringize(LLVersionInfo::instance().getShortVersion(), " (", + LLVersionInfo::instance().getBuild(), ')'); + LLTextBox* forgot_password_text = getChild("forgot_password_text"); forgot_password_text->setClickedCallback(onClickForgotPassword, NULL); @@ -894,9 +894,8 @@ void LLPanelLogin::loadLoginPage() } // Channel and Version - params["version"] = llformat("%s (%ld)", - LLVersionInfo::instance().getShortVersion().c_str(), - LLVersionInfo::instance().getBuild()); + params["version"] = stringize(LLVersionInfo::instance().getShortVersion(), " (", + LLVersionInfo::instance().getBuild(), ')'); params["channel"] = LLVersionInfo::instance().getChannel(); // Grid diff --git a/indra/newview/lltranslate.cpp b/indra/newview/lltranslate.cpp index 1e21c3fa05..6526e1df92 100644 --- a/indra/newview/lltranslate.cpp +++ b/indra/newview/lltranslate.cpp @@ -39,6 +39,7 @@ #include "json/reader.h" #include "llcorehttputil.h" #include "llurlregistry.h" +#include "stringize.h" static const std::string AZURE_NOTRANSLATE_OPENING_TAG("
"); @@ -160,12 +161,12 @@ void LLTranslationAPIHandler::verifyKeyCoro(LLTranslate::EService service, LLSD LLCore::HttpHeaders::ptr_t httpHeaders(new LLCore::HttpHeaders); - std::string user_agent = llformat("%s %d.%d.%d (%ld)", - LLVersionInfo::instance().getChannel().c_str(), - LLVersionInfo::instance().getMajor(), - LLVersionInfo::instance().getMinor(), - LLVersionInfo::instance().getPatch(), - LLVersionInfo::instance().getBuild()); + std::string user_agent = stringize( + LLVersionInfo::instance().getChannel(), ' ', + LLVersionInfo::instance().getMajor(), '.', + LLVersionInfo::instance().getMinor(), '.', + LLVersionInfo::instance().getPatch(), " (", + LLVersionInfo::instance().getBuild(), ')'); initHttpHeader(httpHeaders, user_agent, key); @@ -215,12 +216,12 @@ void LLTranslationAPIHandler::translateMessageCoro(LanguagePair_t fromTo, std::s LLCore::HttpHeaders::ptr_t httpHeaders(new LLCore::HttpHeaders); - std::string user_agent = llformat("%s %d.%d.%d (%ld)", - LLVersionInfo::instance().getChannel().c_str(), - LLVersionInfo::instance().getMajor(), - LLVersionInfo::instance().getMinor(), - LLVersionInfo::instance().getPatch(), - LLVersionInfo::instance().getBuild()); + std::string user_agent = stringize( + LLVersionInfo::instance().getChannel(), ' ', + LLVersionInfo::instance().getMajor(), '.', + LLVersionInfo::instance().getMinor(), '.', + LLVersionInfo::instance().getPatch(), " (", + LLVersionInfo::instance().getBuild(), ')'); initHttpHeader(httpHeaders, user_agent); httpOpts->setSSLVerifyPeer(false); diff --git a/indra/newview/llversioninfo.cpp b/indra/newview/llversioninfo.cpp index 62bfa24e29..9551df7bee 100644 --- a/indra/newview/llversioninfo.cpp +++ b/indra/newview/llversioninfo.cpp @@ -69,7 +69,7 @@ void LLVersionInfo::initSingleton() // fully constructed; such calls don't really belong in the constructor. // cache the version string - version = STRINGIZE(getShortVersion() << "." << getBuild()); + version = stringize(getShortVersion(), ".", getBuild()); } LLVersionInfo::~LLVersionInfo() @@ -91,7 +91,7 @@ S32 LLVersionInfo::getPatch() return LL_VIEWER_VERSION_PATCH; } -S64 LLVersionInfo::getBuild() +U64 LLVersionInfo::getBuild() { return LL_VIEWER_VERSION_BUILD; } diff --git a/indra/newview/llversioninfo.h b/indra/newview/llversioninfo.h index 122bd5c47a..a40042380a 100644 --- a/indra/newview/llversioninfo.h +++ b/indra/newview/llversioninfo.h @@ -61,7 +61,7 @@ public: S32 getPatch(); /// return the build number as an integer - S64 getBuild(); + U64 getBuild(); /// return the full viewer version as a string like "2.0.0.200030" std::string getVersion(); diff --git a/indra/newview/llxmlrpctransaction.cpp b/indra/newview/llxmlrpctransaction.cpp index b851b7ad5c..8ea07fcee0 100644 --- a/indra/newview/llxmlrpctransaction.cpp +++ b/indra/newview/llxmlrpctransaction.cpp @@ -42,6 +42,7 @@ #include "bufferarray.h" #include "llversioninfo.h" #include "llviewercontrol.h" +#include "stringize.h" // Have to include these last to avoid queue redefinition! @@ -384,14 +385,14 @@ void LLXMLRPCTransaction::Impl::init(XMLRPC_REQUEST request, bool useGzip, const httpHeaders->append(HTTP_OUT_HEADER_CONTENT_TYPE, HTTP_CONTENT_TEXT_XML); - std::string user_agent = llformat("%s %d.%d.%d (%ld)", - LLVersionInfo::instance().getChannel().c_str(), - LLVersionInfo::instance().getMajor(), - LLVersionInfo::instance().getMinor(), - LLVersionInfo::instance().getPatch(), - LLVersionInfo::instance().getBuild()); + std::string user_agent = stringize( + LLVersionInfo::instance().getChannel(), ' ', + LLVersionInfo::instance().getMajor(), '.', + LLVersionInfo::instance().getMinor(), '.', + LLVersionInfo::instance().getPatch(), " (", + LLVersionInfo::instance().getBuild(), ')'); - httpHeaders->append(HTTP_OUT_HEADER_USER_AGENT, user_agent); + httpHeaders->append(HTTP_OUT_HEADER_USER_AGENT, user_agent); ///* Setting the DNS cache timeout to -1 disables it completely. //This might help with bug #503 */ -- cgit v1.2.3 From 651353560bfe23b6423ecf7690d86645a71c0cbc Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 17 Oct 2023 14:56:10 -0400 Subject: SL-20476: Don't let the compiler know we intend to crash. clang has gotten smart enough to recognize an inline attempt to store to address zero. Fool it by storing to an address passed as a parameter, and pass nullptr from a different source file. --- indra/llcommon/llerror.cpp | 17 +++++++++++++++-- indra/llcommon/llerror.h | 10 +++++----- 2 files changed, 20 insertions(+), 7 deletions(-) (limited to 'indra') diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 02cb186275..05e719b494 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -1617,5 +1617,18 @@ bool debugLoggingEnabled(const std::string& tag) return res; } - - +void crashdriver(void (*callback)(int*)) +{ + // The LLERROR_CRASH macro used to have inline code of the form: + //int* make_me_crash = NULL; + //*make_me_crash = 0; + + // But compilers are getting smart enough to recognize that, so we must + // assign to an address supplied by a separate source file. We could do + // the assignment here in crashdriver() -- but then BugSplat would group + // all LL_ERRS() crashes as the fault of this one function, instead of + // identifying the specific LL_ERRS() source line. So instead, do the + // assignment in a lambda in the caller's source. We just provide the + // nullptr target. + callback(nullptr); +} diff --git a/indra/llcommon/llerror.h b/indra/llcommon/llerror.h index 020f05e8f5..624a5fb37a 100644 --- a/indra/llcommon/llerror.h +++ b/indra/llcommon/llerror.h @@ -383,11 +383,9 @@ typedef LLError::NoClassInfo _LL_CLASS_TO_LOG; #define LL_NEWLINE '\n' // Use this only in LL_ERRS or in a place that LL_ERRS may not be used -#define LLERROR_CRASH \ -{ \ - int* make_me_crash = NULL;\ - *make_me_crash = 0; \ - exit(*make_me_crash); \ +#define LLERROR_CRASH \ +{ \ + crashdriver([](int* ptr){ *ptr = 0; exit(*ptr); }); \ } #define LL_ENDL \ @@ -466,5 +464,7 @@ typedef LLError::NoClassInfo _LL_CLASS_TO_LOG; // Check at run-time whether logging is enabled, without generating output bool debugLoggingEnabled(const std::string& tag); +// used by LLERROR_CRASH +void crashdriver(void (*)(int*)); #endif // LL_LLERROR_H -- cgit v1.2.3