Use best CMake practices, rm/update outdated code#1416
Open
bmehta001 wants to merge 19 commits intomicrosoft:mainfrom
Open
Use best CMake practices, rm/update outdated code#1416bmehta001 wants to merge 19 commits intomicrosoft:mainfrom
bmehta001 wants to merge 19 commits intomicrosoft:mainfrom
Conversation
Update all actions to latest major versions: - actions/checkout: v1/v2/v3 -> v4 - actions/setup-java: v3 -> v5 - android-actions/setup-android: v2 -> v3 - microsoft/setup-msbuild: v1.1 -> v2 - actions/upload-artifact: v2 -> v4 Fix Android CI: - Use sdkmanager from PATH (setup-android v3 adds it automatically) instead of hardcoded cmdline-tools/7.0/bin directory - Replace hardcoded C:\Users\runneradmin with \C:\Users\bhamehta Fix Windows CI: - Update vs-version from [16,) to [17,) to match VS 2022 runners Fix iOS/macOS CI: - Replace deprecated macos-13 runner with macos-14 - Update iOS deployment target to 14.0 - Update simulator matrix for macos-14/macos-15 runners General: - Add concurrency groups to cancel superseded workflow runs - Updates apply to both active and disabled (.off) workflows Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
xcodebuild on macos-14 runners lists duplicate simulator entries, causing it to pick the 'Any iOS Device' placeholder as the first match. This results in: 'Tests must be run on a concrete device'. Fix by resolving the simulator UUID via xcrun simctl and passing -destination id=<UUID> which is completely unambiguous. This is the recommended approach per xcodebuild docs and actions/runner-images#8621. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Use per-instance ephemeral NSURLSession to prevent stale connection reuse and ensure clean teardown without use-after-destroy crashes - Move session invalidation to CancelAllRequests for safer lifecycle - Fix response body leak in HttpResponseDecoder - Relax flaky test tolerances for CI simulator environments Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…hread - Move m_runningLatency assignment inside LOCKGUARD to prevent concurrent read/write race condition - Clean up remaining task queues on WorkerThread shutdown instead of just logging, preventing memory leak Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove LOG_TRACE from ~Logger() to prevent crash when the logging subsystem is torn down before Logger instances during static destruction (observed on iOS simulator). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Use standard __ANDROID__ macro instead of non-standard ANDROID in HttpClientFactory.cpp - Mark unused log tag with __attribute__((unused)) in HttpClient_Android.cpp - Add #ifndef guards in config-default.h to prevent duplicate macro definitions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Replace 'localhost' with '127.0.0.1' in functional tests to avoid IPv6 resolution failures on some CI environments - Add synchronization loop for storage-full callback in APITest to avoid race condition in assertions - Increase timing tolerances in PalTests for slower CI runners Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The queue cleanup must not run after detach() — the detached thread still needs the shutdown item to exit its event loop. Cleaning up after detach deletes the shutdown signal, causing the thread to hang and then access destroyed members (use-after-free). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
m_runningLatency is read without locks at lines 219 and 376, while written from other threads. Making it std::atomic eliminates the undefined behavior without requiring additional locks (which could deadlock through upload callbacks). The existing locks remain for other shared state they protect. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…sary) Now that m_runningLatency is std::atomic, the write doesn't need to be inside the lock scope. Restore the original position from main to minimize the diff. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move m_runningLatency and m_scheduledUploadTime back to before the LOCKGUARD scope, matching the original code on main. The .cpp now has zero diff from main — all threading fixes are in the .hpp (std::atomic declaration). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
MSVC rejects passing std::atomic<T> to variadic functions (deleted copy constructor). Use explicit .load() to pass the underlying EventLatency value. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
bmehta001
force-pushed
the
bhamehta/code-cleanup
branch
from
6d25b8b to
34e4948
Compare
…-only - Split WARN_FLAGS into GCC and Clang/AppleClang branches so each only gets flags it supports (-Wno-unknown-warning-option is Clang-only) - Add -Wno-reorder as CXX_EXTRA_WARN_FLAGS to suppress member-init order warnings in submodule code - Add -fno-finite-math-only to all non-MSVC REL_FLAGS to restore IEEE 754 compliance (INFINITY macro) needed by sqlite3 and tests when -ffast-math is enabled Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
bmehta001
force-pushed
the
bhamehta/code-cleanup
branch
from
34e4948 to
790fdd0
Compare
bmehta001
changed the title
bmehta001
force-pushed
the
bhamehta/code-cleanup
branch
2 times, most recently
from
e677135 to
c605b8d
Compare
The iOS simulator's network stack on CI runners has higher latency than native builds. The SO_CONNECTION_IDLE socket option is not available, causing NSURLSession to retry connections with delays. - Replace std::this_thread::yield() with PAL::sleep(10) in waitForEvents() to reduce CPU contention on single-core runners - Increase all waitForEvents timeouts from 1-3s to 5-10s - Replace PAL::sleep(2000) with waitForEvents where possible - Increase killSwitchWorks upload wait from 2s to 5s Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The iOS simulator's network stack on CI runners has higher latency than native builds. The SO_CONNECTION_IDLE socket option is not available, causing NSURLSession to retry connections with delays. - Replace std::this_thread::yield() with PAL::sleep(10) in waitForEvents() to reduce CPU contention on single-core runners - Increase all waitForEvents timeouts from 1-3s to 5-10s - Replace PAL::sleep(2000) with waitForEvents where possible - Increase killSwitchWorks upload wait from 2s to 5s Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove references to deleted files that no longer exist: - LogManagerV1.hpp from Shared.vcxitems/.filters - MockILocalStorageReader.hpp from FuncTests.vcxproj.filters - SanitizerBaseTests.cpp incorrect ClInclude from UnitTests.vcxproj - targetver.h from SampleCpp and SampleCppMini .vcxproj/.filters Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Quote path variables in if(EXISTS ...) to prevent policy warnings. Use message(STATUS ...) instead of deprecated format throughout. Simplify test.json copy using configure_file(... COPYONLY). Remove redundant debug messages. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add v145 for VS 2026 (18.0) and use DefaultPlatformToolset as fallback instead of hardcoded v141. Replace hardcoded v141/VCToolsVersion in build.MIP.props with DefaultPlatformToolset for portability across VS versions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Root CMakeLists.txt:
- Add LANGUAGES C CXX to project() declaration
- Use message(STATUS ...) throughout instead of message("-- ...")
- Remove empty compiler ID blocks (Intel, Clang with only comments)
- Remove commented-out bondlite test block
- Remove stale FIXME/TODO comments in package section
- Update MATSDK_API_VERSION from 3.4 to 3.10
lib/CMakeLists.txt:
- Use message(STATUS ...) throughout
- Quote all if(EXISTS ...) path variables
- Remove duplicate include/public in include_directories
- Fix message(FATAL_ERROR, ...) comma bug (treated as two args)
- Remove commented-out linker code blocks
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
bmehta001
force-pushed
the
bhamehta/code-cleanup
branch
from
c605b8d to
99bc4c0
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Use best CMake practices and remove/update outdated settings