explicitly link stdlib

[andiwand]

No description provided.

[claude]

Claude finished @andiwand's task —— View job

---

Code Review for PR #427: explicitly link stdlib

Todo List:

• Read app/build.gradle to understand the stdlib linking change
• Check for related CMake/native build configuration
• Analyze the impact of explicit stdlib linking
• Provide comprehensive code review feedback

Summary of Changes

This PR makes two changes to app/build.gradle:

1. Adds -DANDROID_STL=c++_shared to the CMake arguments
2. Removes the app.opendocument:wvware-android:1.2.11 dependency

Review Feedback

1. Code Quality and Best Practices ✅

The addition of -DANDROID_STL=c++_shared is a good practice for Android NDK projects:

• Correct placement: The argument is properly added to the CMake arguments list
• Consistency: This aligns with the Conan profile (conanprofile.txt:9) which already specifies compiler.libcxx=c++_shared
• Explicit configuration: Making the STL choice explicit in the build configuration improves maintainability

2. Potential Bugs or Issues ⚠️

Critical Issue: The removal of wvware-android dependency appears unrelated to the PR title and could break functionality:

• wvware-android is used by WvwareDocLoader for MS Word document processing (per CLAUDE.md)
• Removing this dependency will likely cause runtime crashes when users try to open Word documents
• This change is not mentioned in the PR title or description

Recommendation: This dependency removal should be reverted or moved to a separate PR with proper justification.

3. Performance Considerations ✅

The STL linking change has minimal performance impact:

• c++_shared is the recommended STL for apps with multiple shared libraries
• Shared STL reduces APK size when multiple native libraries use the C++ standard library
• No performance degradation expected

4. Security Concerns ✅

No security issues identified:

• Using the standard Android STL is secure
• The explicit configuration reduces ambiguity

5. Test Coverage ❓

Concerns:

• No tests appear to be added or modified with this change
• The removal of wvware-android will likely cause existing tests to fail
• Recommend running ./gradlew connectedAndroidTest to verify functionality

Recommendations

1. Separate the changes: The STL configuration and dependency removal should be in separate PRs
2. Add PR description: Explain why these changes are needed
3. Test impact: Run the test suite to ensure Word document processing still works
4. Consider alternatives: If wvware-android is being replaced, document the migration path

Conclusion

While the explicit STL configuration is a positive change, the unexplained removal of the wvware-android dependency is concerning and could break core functionality. I recommend splitting this into two PRs or reverting the dependency removal.