Add -PHAVE_CS4 Gradle property for Android CS4 builds (follow-up to #1476)#1477
Closed
bmehta001 wants to merge 2 commits into
Closed
Add -PHAVE_CS4 Gradle property for Android CS4 builds (follow-up to #1476)#1477bmehta001 wants to merge 2 commits into
bmehta001 wants to merge 2 commits into
Conversation
Map the -PHAVE_CS4 / -PHAVE_CS4_FULL Gradle properties (and the HAVE_CS4 / HAVE_CS4_FULL environment variables) to the MATSDK_HAVE_CS4 / MATSDK_HAVE_CS4_FULL CMake options, mirroring the existing USE_ROOM dedicated parameter. This is the discoverable, propagating way to enable Common Schema 4.0 on Android instead of stuffing a raw -DHAVE_CS4=1 into CXXFLAGS. Depends on microsoft#1476 (adds the MATSDK_HAVE_CS4 option); inert until that lands. - maesdk/build.gradle: -PHAVE_CS4[_FULL] -> -DMATSDK_HAVE_CS4[_FULL]=ON - gradle.properties: document the dedicated property Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds dedicated, discoverable Gradle properties for Android builds to enable Common Schema 4.0 (CS4) by mapping -PHAVE_CS4=1 / -PHAVE_CS4_FULL=1 (or corresponding env vars) into CMake options, avoiding the need to inject raw -DHAVE_CS4=1 via CXXFLAGS and aligning with the propagating CMake option introduced in PR #1476.
Changes:
- Add Gradle property/env var handling in the Android build to pass
-DMATSDK_HAVE_CS4=ON/-DMATSDK_HAVE_CS4_FULL=ONto CMake. - Document the new Gradle properties in
lib/android_build/gradle.properties.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| lib/android_build/maesdk/build.gradle | Reads HAVE_CS4 / HAVE_CS4_FULL from Gradle properties or environment and maps them to CMake options. |
| lib/android_build/gradle.properties | Documents the new -PHAVE_CS4=1 / -PHAVE_CS4_FULL=1 build-time properties and keeps the CXXFLAGS escape hatch documented. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+34
to
+41
| String haveCs4 = project.findProperty("HAVE_CS4") ?: System.getenv("HAVE_CS4") ?: "" | ||
| if (haveCs4.equalsIgnoreCase("1") || haveCs4.equalsIgnoreCase("true") || haveCs4.equalsIgnoreCase("on")) { | ||
| args.add("-DMATSDK_HAVE_CS4=ON") | ||
| } | ||
| String haveCs4Full = project.findProperty("HAVE_CS4_FULL") ?: System.getenv("HAVE_CS4_FULL") ?: "" | ||
| if (haveCs4Full.equalsIgnoreCase("1") || haveCs4Full.equalsIgnoreCase("true") || haveCs4Full.equalsIgnoreCase("on")) { | ||
| args.add("-DMATSDK_HAVE_CS4_FULL=ON") | ||
| } |
Contributor
Author
There was a problem hiding this comment.
Fixed in b4fd4fe. When -PHAVE_CS4_FULL (or the env var) is set, the mapping now also adds -DMATSDK_HAVE_CS4=ON (cs4 = cs4Full || cs4), so it does not rely on the CMake-side implication.
build.gradle: when -PHAVE_CS4_FULL (or env) is set, also add -DMATSDK_HAVE_CS4=ON so the Gradle mapping does not rely on the CMake-side implication (cs4 = cs4Full || cs4). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
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.
Follow-up to #1476. Adds a discoverable, dedicated Gradle property for enabling Common Schema 4.0 on Android, instead of stuffing a raw
-DHAVE_CS4=1intoCXXFLAGS.Change
lib/android_build/maesdk/build.gradlemaps:-PHAVE_CS4=1(or theHAVE_CS4env var) →-DMATSDK_HAVE_CS4=ON-PHAVE_CS4_FULL=1(orHAVE_CS4_FULLenv) →-DMATSDK_HAVE_CS4_FULL=ONThis mirrors the existing
USE_ROOMdedicated-parameter pattern, and uses the CMake option from #1476 — soHAVE_CS4is applied as a PUBLIC compile definition and propagates to consumers (the raw-CXXFLAGSpath does not).gradle.propertiesis updated to document the property.The existing
CXXFLAGSpath is unchanged, and with no property set the build is byte-for-byte the same — so this is inert until the property is used.Depends on #1476
This maps to the
MATSDK_HAVE_CS4CMake option added in #1476. Until that merges,-DMATSDK_HAVE_CS4=ONis a no-op; please merge #1476 first.Deferred: vcpkg
cs4featureThe third CS4 follow-up — a
cs4feature on thecpp-client-telemetryvcpkg port — is intentionally not included. Both the official port (microsoft/vcpkg#52316) and the in-repo overlay build from thev3.10.161.1release tag, which does not containMATSDK_HAVE_CS4(it's #1476). A feature there would pass-DMATSDK_HAVE_CS4=ONto a source that ignores it — silently doing nothing. It should be added when the port bumps to a release that includes #1476 (e.g. via thevcpkg-release-bumpworkflow in #1475).