Skip to content

Bhamehta/code cleanup#2

Open
bmehta001 wants to merge 31 commits into
mainfrom
bhamehta/code-cleanup
Open

Bhamehta/code cleanup#2
bmehta001 wants to merge 31 commits into
mainfrom
bhamehta/code-cleanup

Conversation

@bmehta001
Copy link
Copy Markdown
Owner

@bmehta001 bmehta001 commented Mar 18, 2026

Fixes runtime thread-safety, shutdown-safety, and response-lifetime issues that affect normal SDK operation. Split out from microsoft#1415 per reviewer request so runtime behavior changes stay separate from CI/build/test fixes.

Fix HTTP cancellation and callback tracking

HttpClient_Apple.mm

  • Scope cancellation to the current request’s m_dataTask instead of canceling every task on the shared static NSURLSession.
  • Fix the CancelAllRequests() wait loop so m_requests.empty() is checked while holding m_requestsMtx.

HttpClientManager.cpp

  • Fix the cancelAllRequests() wait loop so m_httpCallbacks.empty() is checked while holding m_httpCallbacksMtx.

Fix HTTP response lifetime on abort/network-failure paths

HttpResponseDecoder.cpp

  • Preserve ctx->httpResponse through requestAborted(ctx) and temporaryNetworkFailure(ctx) so downstream storage/statistics handlers can still read status and headers.
  • Avoid leaking aborted/network-failure responses by keeping ownership with EventsUploadContext, whose clear() path deletes the response.

HttpResponseDecoderTests.cpp

  • Add regression coverage that aborted and network-failure decode routes still receive the response object with result/status intact.

Fix WorkerThread shutdown safety

WorkerThread.cpp

  • Add an explicit shutdown gate so late Queue() calls are rejected once teardown starts.
  • Enqueue the shutdown sentinel only once under the queue lock.
  • Clean up pending queued/timer tasks only after a successful join(), and avoid cleanup after detach() because the worker may still be running.
  • Log join/detach failures instead of silently swallowing all exceptions.

Make TransmissionPolicyManager scheduling consistently mutex-guarded

TransmissionPolicyManager.cpp / .hpp

  • Guard m_isUploadScheduled, m_runningLatency, and m_scheduledUploadTime consistently with m_scheduledUploadMutex.
  • Avoid holding m_scheduledUploadMutex across potentially blocking cancellation during stop/shutdown.
  • Keep force/zero-delay no-wait cancellation under the scheduler mutex so a competing delayed schedule cannot suppress an immediate upload.
  • Use an explicit std::chrono::milliseconds value in the bandwidth-controller reschedule path.

TransmissionPolicyManagerTests.cpp

  • Add regression coverage for the force/zero-delay scheduling race.

Fix Logger static-destruction-order crash

Logger.cpp

  • Remove destructor logging from Logger::~Logger() because it can run after logging infrastructure has already been destroyed, causing crashes during static teardown.

Validation

  • HttpResponseDecoderTests.* and TransmissionPolicyManagerTests.ForceScheduleRetainsImmediateUploadWhenCancelBlocks passed.
  • Full host UnitTests passed: 485/485

@bmehta001 bmehta001 self-assigned this Mar 18, 2026
@bmehta001 bmehta001 force-pushed the bhamehta/code-cleanup branch 20 times, most recently from 9a5a166 to 2feeb94 Compare March 19, 2026 05:40
@bmehta001 bmehta001 force-pushed the bhamehta/code-cleanup branch 4 times, most recently from e303041 to 9a91839 Compare April 1, 2026 16:55
@bmehta001 bmehta001 force-pushed the bhamehta/code-cleanup branch 5 times, most recently from b30c9b8 to 95502d5 Compare April 29, 2026 21:29
bmehta001 and others added 10 commits May 22, 2026 15:02
- Modernize CMakeLists.txt: flatten nesting, deduplicate, use
  consistent quoting and variable patterns
- Remove stale header references from vcxproj and vcxitems files
- Simplify test CMakeLists.txt files
- Fix CMake conventions in packaging scripts

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep the before.targets toolset selection deterministic on newer
Visual Studio hosts, but only set the MIP props fallback when a
consumer has not already chosen a toolset. While addressing the
MSBuild review comments, also point the optional module test
conditions and source paths at the real lib/modules locations and
use CPACK_PACKAGE_FILE_NAME for the RPM status message.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Quote the optional ECS test.json configure_file input and output paths so CMake handles source or build directories that contain spaces.

Files changed:

- tests/functests/CMakeLists.txt

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PR microsoft#1416 inadvertently regressed PR microsoft#1415 by removing
-fno-finite-math-only from the three Unix REL_FLAGS branches and adding
-Wno-nan-infinity-disabled to silence the Clang diagnostic. That left
release builds using -ffast-math without preserving the NaN/Inf
semantics needed by nlohmann::json and SQLite paths.

Remove -ffast-math entirely from the GCC, AppleClang, and generic Clang
release flags rather than relying on -fno-finite-math-only to partially
undo it. This SDK is not floating-point compute-bound; its hot paths are
string, Bond/JSON serialization, HTTP I/O, and SQLite reads/writes.

Avoiding -ffast-math:
- preserves std::isnan/std::isinf behavior for JSON and storage code,
- avoids compiler/runtime fast-math side effects such as x86 GCC's
  crtfastmath.o changing MXCSR FTZ/DAZ behavior process-wide, and
- aligns with SQLite's guidance to avoid fast-math.

This subsumes microsoft#1415's partial mitigation and aligns with microsoft#1392's intent,
extended to GCC because GCC fast-math has the same broad assumptions and
runtime side effects. Also remove -Wno-nan-infinity-disabled because the
warning should not be suppressed once the cause is gone.

Validation:
- CMake Release build of UnitTests on macOS arm64 (AppleClang 21).
- UnitTests passed on macOS arm64.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Avoid carrying a confusing note about warning flags that are not used, and keep CMakeLists.txt focused on active compiler settings.

Files changed:

- CMakeLists.txt

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@bmehta001 bmehta001 force-pushed the bhamehta/code-cleanup branch from 1d09c10 to 1cfe326 Compare May 22, 2026 20:09
pablo-msft and others added 19 commits May 27, 2026 19:31
Copilot review on PR microsoft#1416 flagged 7 unquoted ${CMAKE_SOURCE_DIR}/...
paths inside list(APPEND SRCS ...) calls. CMake splits unquoted
arguments on whitespace, so a checkout path containing spaces (very
common on Windows: C:\Users\First Last\source\...) would fragment a
single source path into multiple list elements and the test wouldn't
compile.

Quote every ${CMAKE_SOURCE_DIR}/.../*.cpp path inside
list(APPEND SRCS ...) in both tests/unittests/CMakeLists.txt and
tests/functests/CMakeLists.txt (23 paths total, including the
privacyguard + sanitizer blocks Copilot didn't cite but that share the
same hazard).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot review on PR microsoft#1416 flagged that mapping VisualStudioVersion >= 18.0
to PlatformToolset v145 is a guess: VS 2025/2026/etc. has not shipped, the
toolset moniker for that release may differ, and pinning to a non-existent
toolset name causes MSBuild to fail early with "unknown PlatformToolset"
on any machine that does pick up that VS version.

Drop the v145 mapping from both before.targets and build.MIP.props. Future
VS versions fall through to the v141 fallback (same behavior as before
PR microsoft#1416, just one less wrong-toolset-name failure mode). Add explicit
mappings for new toolsets as they actually ship.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The top-level CMakeLists.txt asks for C++14 (CMAKE_CXX_STANDARD 14, set in
2018) but then re-injects -std=c++11 into CMAKE_CXX_FLAGS for both Release
and Debug Unix builds (set in 2019 when gcc-4.x / gcc-5.x C++14 support was
still spotty). That explicit flag wins, so the Unix builds have actually
been C++11 ever since, contradicting the stated standard.

This is a vestige: gcc-5+, clang-3.4+, and MSVC 2015+ all support C++14
fully, and Android (lib/android_build/) already sets CMAKE_CXX_STANDARD 14
explicitly. Removing the override aligns the Unix CMake builds with the
intended standard.

- Drop the stale gcc-4.x/5.x caveat comment and add CMAKE_CXX_STANDARD_REQUIRED
  ON so the build fails loudly on toolchains that cannot do C++14, rather
  than silently falling back.
- Drop the -std=c++11 fragment from both REL and DBG CMAKE_CXX_FLAGS lines.
  C and C11 are left alone (C11 remains the C floor).

Not touched in this commit:
- examples/**/CMakeLists.txt and wrappers/obj-c/CMakeLists.txt also pin
  -std=c++11. Those are standalone sample/wrapper build scripts not wired
  via add_subdirectory and not built by any CI workflow; their pins are
  intentional statements of each sample's minimum and can stay.
- This commit does not bump the project to C++17. Cleanup ideas like
  cpp_client_telemetry_modules#302 (std::string::data() -> char*) require
  C++17 and are still out of scope.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… install (microsoft#1451)

* ci: add least-privilege permissions to remaining workflows

Adds top-level `permissions: { contents: read }` to the six workflow
files on `main` that lacked an explicit GITHUB_TOKEN scope:

  - .github/workflows/build-android.yml
  - .github/workflows/build-ios-mac.yml
  - .github/workflows/build-posix-latest.yml
  - .github/workflows/build-ubuntu-2204.yml
  - .github/workflows/build-windows-vs2022.yaml
  - .github/workflows/test-win-latest.yml

All six only check out source and run builds or tests; none post PR
comments, deploy artifacts via the token, or otherwise need write
scopes. `actions/upload-artifact@v4` (build-android.yml) uses its own
per-run SAS URL and does not require GITHUB_TOKEN scopes.

Carry-forward from PR microsoft#1450, which hardened spellcheck.yml the same
way, and mirrors the analogous modules-repo commit decc96501. Each
file gets an explanatory comment naming the CodeQL rule it satisfies
(actions/missing-workflow-permissions) if Actions analysis is later
enabled in this repo's code-scanning config. No behavior change.

Already-permissioned workflows (codeql-analysis.yml, deploy-docs-pages.yml)
are not touched.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* ci(spellcheck): pin misspell + add least-privilege permissions

Three hardening fixes for .github/workflows/spellcheck.yml, ported
from the same audit applied to the sister modules repo:

1. Add top-level 'permissions: contents: read'. Without an explicit
   block, the GITHUB_TOKEN inherits the repo default (typically
   read+write across most APIs), which is far more than misspell
   needs. misspell only walks .md/.txt/lib/tests files and prints
   typos to stdout; it never comments on PRs, posts statuses, or
   writes packages. This also satisfies CodeQL rule
   'actions/missing-workflow-permissions' if Actions analysis is
   enabled here later (it isn't in the current codeql-analysis.yml
   matrix, but no reason to wait for that).

2. Replace the unpinned bootstrap install with a pinned tarball
   download. The prior 'curl https://git.io/misspell | sh' pattern
   executed a shell script from the master branch of an external
   repo via a git.io redirect — a supply-chain risk that also made
   CI non-reproducible. Now we download a specific release artifact
   (v0.3.4) directly from GitHub releases.

3. Verify the tarball SHA256 against the published checksum
   'afd95caf1eecc72ff382791e00b3b11523a20b0579d95e2295c1c043688743d5'
   (linux_64bit). Any tampering or unexpected upstream change will
   fail CI rather than silently execute.

Scope of misspell unchanged: still scans .md, .txt, examples/**, all
of lib/ (excluding json.hpp), and tests/**.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Reduce C++ CodeQL build scope

Build a single representative x64 Release native configuration instead of the full Windows matrix so CodeQL extracts a smaller database and avoids standard-runner memory exhaustion.

Files changed:

- .github/workflows/codeql-analysis.yml

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Move CodeQL C++ build into script

Keep the reduced CodeQL C++ target list in a dedicated batch file instead of embedding it directly in the workflow, as requested in review.

Files changed:

- .github/workflows/codeql-analysis.yml

- build-codeql-cpp.bat

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Fix Windows OS build version reporting

Use servicing-aware registry values so desktop telemetry reports the current Windows build instead of stale BuildLabEx data after OS updates.

Files changed:

- lib/pal/desktop/WindowsDesktopSystemInformationImpl.cpp

- tests/unittests/PalTests.cpp

Fixes microsoft#1407

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Add deterministic Windows OS version source tests

Refactor the Windows OS full-version composition into a testable helper so registry and Rtl build source precedence can be covered without depending on the host machine state.

The new PAL tests cover CurrentBuildNumber precedence, CurrentBuild fallback, Rtl build fallback, and missing UBR formatting so future key or fallback regressions are caught before merge.

Files changed:

- lib/pal/desktop/WindowsDesktopSystemInformationImpl.cpp

- tests/unittests/PalTests.cpp

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Cover zero Windows servicing build revision

Add a deterministic PAL test proving that UBR value 0 is still included when the source reports it as present. This keeps zero-value handling separate from the missing-UBR fallback.

Files changed:

- tests/unittests/PalTests.cpp

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Use RtlGetVersion build number; combine with registry UBR

Per Windows-team guidance on issue microsoft#1407: the RtlGetVersion() API is the
authoritative source for the running OS build number, and the registry
CurrentBuildNumber / CurrentBuild values are only useful for offline
reads of another Windows installation. For the running OS the API and
registry are always in sync, so prefer the API. UBR is registry-only and
is always safe to combine with the kernel-provided build number.

Drop the registry CurrentBuildNumber / CurrentBuild precedence chain and
the now-unused getCurrentVersionStringValue helper. Format the OS full
version directly from rtlOsvi.dwBuildNumber plus the optional UBR.

Update PalTests to cover the simplified formatter (UBR present, UBR
missing, zero UBR present), replacing the obsolete source-precedence
tests.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Migrate Apple reachability from SCNetworkReachability to NWPathMonitor

Replace the deprecated SCNetworkReachability APIs with NWPathMonitor
for modern Apple deployment targets (iOS 12+, macOS 10.14+). The legacy
SCNetworkReachability path is retained behind a compile-time check for
older targets.

Changes:
- NetworkInformationImpl.mm: refactor to use NWPathMonitor as the
  primary reachability mechanism, with SCNetworkReachability as fallback
  for older deployment targets only
- ODWReachability.h/m: add NWPathMonitor-based implementation gated on
  availability, keeping SCNetworkReachability for backward compatibility
- Remove dead private header imports from tests

This eliminates the -Wdeprecated-declarations build failures on
Xcode 26.4+ without needing pragma suppressions.

Fixes microsoft#1425

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Restore ODWReachabilityTests includes to keep microsoft#1431 focused

PR microsoft#1431 should not carry changes in ODWReachabilityTests.mm.
Restore the socket header imports so the branch only contains the
reachability implementation changes.

Files changed:
- tests/unittests/obj-c/ODWReachabilityTests.mm

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Restore ODWReachability.m header section to match main

Keep PR microsoft#1431 focused on the reachability implementation changes by
reverting the top-of-file/header-area edits in ODWReachability.m.

Files changed:
- third_party/Reachability/ODWReachability.m

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Use NWPathMonitor in ODWReachability

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Clarify modern WWAN reachability handling

Make the modern ODWReachability WWAN path explicit so iOS 12+
builds unambiguously use the NWPathMonitor-backed state, while the
legacy SCNetworkReachability fallback remains only for older Apple
deployment targets.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Avoid blocking Apple reachability construction

Modern reachability should not synchronously resolve DNS or reject the generic internet reachability address, and the path monitor context must not hold a stale raw owner pointer. Also avoid blocking the main thread while waiting for the first NWPathMonitor snapshot.

Files changed:
- third_party/Reachability/ODWReachability.m

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Gate ODWReachability NWPathMonitor calls by availability

Annotate the modern-path helpers (`ODWModernPathIsReachable`,
`ensureModernPathMonitor`, `awaitModernPathSnapshot`,
`handleModernPathUpdate:`, `notifyModernPathChange`) with
`API_AVAILABLE(macos(10.14), ios(12.0))` so that compiling against an
older deployment target no longer triggers
`-Wunguarded-availability-new` errors under `-Werror`.

Also fix a latent bug in `isReachableViaWWAN` where the modern-path
snapshot was computed before the `if (@available(...))` guard. On
macOS 10.10 / iOS 10.0 deployment targets running on a host that
lacks NWPathMonitor, this would invoke unavailable Network framework
APIs. The modern-path branch is now only taken inside the
`@available` block on legacy builds, mirroring the structure used in
`isReachableViaWiFi`.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Capture ODWReachability semaphore before signal/wait

`-handleModernPathUpdate:` and `-awaitModernPathSnapshot` previously
read `self.initialPathSemaphore` more than once across the nil-check
and the matching `dispatch_semaphore_signal`/`dispatch_semaphore_wait`
call. `-stopNotifier` clears the property from an arbitrary thread, so
the second read could observe nil and pass it into libdispatch (which
crashes on a nil semaphore).

Capture the semaphore into a strong local once. Under ARC the local
retains the dispatch_semaphore_t for the duration of the call, so a
concurrent stop will no longer race the signal/wait.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Address reachability review comments

Preserve host/address-specific reachability semantics with SCNetworkReachability while keeping Network.framework for general path monitoring.

Avoid waiting for the first NWPathMonitor update on the monitor queue, and use platform-specific Apple availability gates.

Files changed:

- third_party/Reachability/ODWReachability.h

- third_party/Reachability/ODWReachability.m

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Avoid unguarded NWPath status init

Leave the Network.framework status initialization in modern monitor setup so older Apple deployment-target builds do not touch an availability-gated symbol from init.

Files changed:

- third_party/Reachability/ODWReachability.m

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Limit reachability gates to supported Apple targets

Remove tvOS and watchOS deployment-target handling because the SDK does not support those targets; keep the availability gates focused on iOS and macOS.

Files changed:

- third_party/Reachability/ODWReachability.h

- third_party/Reachability/ODWReachability.m

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Validate +reachabilityWithAddress: inputs

Reject the IPv4 unspecified wildcard (INADDR_ANY / 0.0.0.0), the IPv6
unspecified wildcard (in6addr_any / ::), and any sa_family that is not
AF_INET or AF_INET6 from the public +reachabilityWithAddress: entry
point. These are not routable host addresses, so creating an SC ref for
them produces an ambiguous reachability probe rather than a per-host
result.

The legacy +reachabilityForInternetConnection fallback (only reached on
deployment targets older than macOS 10.14 / iOS 12) still needs the
'probe any internet' 0.0.0.0 sentinel, so it now creates the SC ref
inline and bypasses the public validator.

Also document why hostname-based reachability still routes through
SCNetworkReachabilityCreateWithName: NWPathMonitor has no public
hostname-targeted monitoring API.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Address PR microsoft#1431 review comments

- Unconditionally set sockaddr_in::sin_len / sockaddr_in6::sin6_len in
  +reachabilityWithAddress: after copying the caller-supplied address.
  Callers (including the unit tests in ODWReachabilityTests.mm) don't
  reliably initialize the BSD length byte, so the previous "fix it only
  when zero" pattern let garbage values through and caused
  SCNetworkReachabilityCreateWithAddress to fail on otherwise-valid
  IPv4/IPv6 inputs.

- Restructure +reachabilityForInternetConnection and
  +reachabilityForLocalWiFi so the legacy SCNetworkReachability fallback
  code is wrapped in #if ODW_LEGACY_REACHABILITY_REQUIRED instead of
  living below a modern early-return as unreachable code. This actually
  compiles out the deprecated SC creation APIs on modern targets, which
  is the stated goal of the PR, rather than relying on dead-code +
  pragma suppression.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Remove unused checkNetworkReachability helper

This URLSession-based method was the early URLSession reachability probe
and is no longer referenced by anything after migrating the public
reachability surface to NWPathMonitor / the legacy SCNetworkReachability
notifier. Drop the stale definition so the modern Apple path doesn't
carry dead code.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Document hostname-constructor semantic change on modern Apple targets

Add a header-level docstring explaining that on modern targets
(iOS 12+ / macOS 10.14+), +reachabilityWithHostname: and
+reachabilityWithAddress: no longer issue an NSURLSession HTTPS probe to
the host's URL when -isReachable / -isReachableViaWiFi are queried.
Instead they return SCNetworkReachabilityGetFlags state on the SC ref
the constructor created.

This is the same OS-level reachability semantics the legacy code path
has always used, but it is a behavior change compared to the pre-PR
modern path that performed a real HTTPS round-trip. Callers that need
authoritative per-endpoint reachability should probe the endpoint
themselves rather than relying on -isReachable on these instances.

Document that the SDK's own production network detection
(NetworkInformationImpl) uses +reachabilityForInternetConnection on
legacy targets and nw_path_monitor_create() directly on modern targets,
so the SDK's own behavior is unaffected.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Simplify ODWReachability ARC-only code

Drop manual-reference-counting fallback code from the new NWPathMonitor context now that the Apple builds compile this file with ARC.

Files changed:

- third_party/Reachability/ODWReachability.m

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Address Apple reachability review comments

Synchronize NWPathMonitor snapshot reads, unblock pending snapshot waits when stopping the monitor, and guard against monitor creation failure in the SDK network detector.

Files changed:

- third_party/Reachability/ODWReachability.m

- lib/pal/posix/NetworkInformationImpl.mm

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix Apple reachability monitor races

Serialize NWPathMonitor initialization and use a dispatch group so every caller waiting for the first path snapshot is released when the snapshot arrives or monitoring stops.

Files changed:

- third_party/Reachability/ODWReachability.m

- lib/pal/posix/NetworkInformationImpl.mm

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix Reachability hostname parsing

Copilot: host:port inputs were parsed as a scheme with no host. Parse by prepending https:// before NSURLComponents when no explicit scheme separator is present.

Copilot: invalid.hostname is syntactically valid under SCNetworkReachabilityCreateWithName. Update tests to distinguish malformed host URLs from unresolved hostnames.

Files changed:

- third_party/Reachability/ODWReachability.m

- tests/unittests/obj-c/ODWReachabilityTests.mm

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Clarify Reachability ARC build behavior

Copilot: ODWReachability.m could look non-ARC under CMake because ARC was only set in CXX flags. Compile the vendored Objective-C source with -fobjc-arc explicitly.

Copilot: rename the unresolved-hostname test to describe constructor acceptance rather than DNS reachability.

Files changed:

- lib/CMakeLists.txt

- tests/unittests/obj-c/ODWReachabilityTests.mm

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Remove blocking Reachability notifier startup

Copilot: delete the dead URLSession response helper left behind after removing endpoint probes.

Copilot: make modern startNotifier match legacy behavior by wiring callbacks and returning without waiting for the first NWPathMonitor snapshot.

Copilot: add a start/stop notifier test that verifies startup returns promptly.

Files changed:

- third_party/Reachability/ODWReachability.m

- tests/unittests/obj-c/ODWReachabilityTests.mm

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Match repo style in reachability files

Restore the access-specifier indentation in NetworkInformationImpl.mm to the 4-space convention used on main.

Restore the trailing blank line at the end of lib/CMakeLists.txt accidentally dropped during the ARC compile-flags patch.

Realign one stray line in ODWReachability.m reachabilityFlags() to match surrounding column alignment.

Files changed:

- lib/CMakeLists.txt

- lib/pal/posix/NetworkInformationImpl.mm

- third_party/Reachability/ODWReachability.m

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Bump default iOS deployment target to 12.0

Align build-ios.sh with the README support matrix (iOS 12+) so default builds compile out the legacy SCNetworkReachability code path that ODW_LEGACY_REACHABILITY_REQUIRED gates. Borrowed from microsoft#1371.

Files changed:

- build-ios.sh

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Remove internal ODWReachability helper

ODWReachability is internal-only, so keep the SDK on direct NWPathMonitor instead of maintaining a compatibility wrapper around the vendored Reachability snapshot.

Remove the vendored Reachability sources, tests, CMake wiring, Xcode references, and OSS manifest entries. Unsupported pre-iOS-12 / pre-macOS-10.14 runtimes now leave network state as Unknown instead of using SCNetworkReachability.

Files changed:

- lib/pal/posix/NetworkInformationImpl.mm

- lib/CMakeLists.txt

- tests/unittests/unittests-ios.xcodeproj/project.pbxproj

- tests/unittests/obj-c/ODWReachabilityTests.mm

- third_party/Reachability/*

- docs/List-of-OSS-Components.md

- third_party/cgmanifest.json

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Baiju Meswani <baijumeswani@gmail.com>
Derive package version components from MATSDK_BUILD_VERSION so CPack packages stay aligned with the SDK version.

Use PROJECT_SOURCE_DIR and PROJECT_BINARY_DIR for test CMake paths so the SDK remains subproject-friendly.

Files changed:

- CMakeLists.txt

- tools/MakeDeb.cmake

- tools/MakeRpm.cmake

- tools/MakeTgz.cmake

- tests/functests/CMakeLists.txt

- tests/unittests/CMakeLists.txt

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Validate MATSDK_BUILD_VERSION before deriving CPack version components and remove now-dead DAYNUMBER calculations from package scripts.

Files changed:

- CMakeLists.txt

- tools/MakeDeb.cmake

- tools/MakeRpm.cmake

- tools/MakeTgz.cmake

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The previous merge from main left a conflict marker block in lib/CMakeLists.txt's trailing commented-link section. Remove the marker block and keep the cleaned-up file content.

Files changed:

- lib/CMakeLists.txt

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep package script version cleanup out of the CMake cleanup PR; package version alignment can be handled separately before the next release.

Files changed:

- CMakeLists.txt

- tools/MakeDeb.cmake

- tools/MakeRpm.cmake

- tools/MakeTgz.cmake

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add an explicit VS2026/v145 command-line build entry alongside the existing VS2022/v143 script so Windows consumers can choose the right supported toolset without guessing from MSBuild internals.

Files changed:
- docs/cpp-start-windows.md
- build-all-v145.bat
- tools/vcvars.cmd
- Solutions/before.targets

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants