Skip to content

[BUILD] Fixes compiling problems in NDK r27 #3517

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 4, 2025

Conversation

owent
Copy link
Member

@owent owent commented Jul 4, 2025

Fixes #3516

Changes

  • Fixes compiling problems in NDK r27

For significant contributions please make sure you have completed the following items:

  • CHANGELOG.md updated for non-trivial changes
  • Unit tests have been added
  • Changes in public API reviewed

@Copilot Copilot AI review requested due to automatic review settings July 4, 2025 03:06
@owent owent requested a review from a team as a code owner July 4, 2025 03:06
Copy link

netlify bot commented Jul 4, 2025

Deploy Preview for opentelemetry-cpp-api-docs canceled.

Name Link
🔨 Latest commit 816455b
🔍 Latest deploy log https://app.netlify.com/projects/opentelemetry-cpp-api-docs/deploys/6867d065600c570008769c78

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR addresses compilation issues in NDK r27 by reorganizing includes and correcting std::shared_ptr<FILE> usage for file handles.

  • Moved the nlohmann/json.hpp include to eliminate duplicates.
  • Replaced the invalid std::make_shared<FILE>() with a direct std::shared_ptr<FILE> allocation using fclose as a deleter.
Comments suppressed due to low confidence (2)

exporters/otlp/src/otlp_file_client.cc:1296

  • [nitpick] The variable name of is ambiguous; consider renaming it to file_handle or file_ptr for better clarity.
    std::shared_ptr<FILE> of = std::shared_ptr<std::FILE>(new_file, fclose);

exporters/otlp/src/otlp_file_client.cc:1239

  • Consider adding a unit test to verify that the file handle is correctly opened and closed under NDK r27 to prevent future regressions.
    }

Copy link

codecov bot commented Jul 4, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.96%. Comparing base (f4897b2) to head (816455b).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3517      +/-   ##
==========================================
+ Coverage   89.95%   89.96%   +0.02%     
==========================================
  Files         219      219              
  Lines        7051     7051              
==========================================
+ Hits         6342     6343       +1     
+ Misses        709      708       -1     

see 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@marcalff marcalff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Commit owned by owent and not by copilot, so eacyCLA is clear here.

Ok to merge, with minor fix on std::FILE versus FILE.

@marcalff
Copy link
Member

marcalff commented Jul 4, 2025

@owent Thanks for the std fixes. Will merge once CI completes.

@marcalff marcalff changed the title Fixes compiling problems in NDK r27 [BUILD] Fixes compiling problems in NDK r27 Jul 4, 2025
@marcalff marcalff merged commit b9db1b3 into open-telemetry:main Jul 4, 2025
70 checks passed
@owent owent deleted the fixes_3516 branch July 10, 2025 03:13
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.

Can not compile with Android NDK r27.2
3 participants