Skip to content

internal/crosscompile:update clang with older glibc & more small size#1275

Merged
xushiwei merged 2 commits intogoplus:mainfrom
luoliwoshang:update/clang
Sep 10, 2025
Merged

internal/crosscompile:update clang with older glibc & more small size#1275
xushiwei merged 2 commits intogoplus:mainfrom
luoliwoshang:update/clang

Conversation

@codecov
Copy link

codecov bot commented Sep 8, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.28%. Comparing base (f41badd) to head (d2a021c).
⚠️ Report is 78 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1275   +/-   ##
=======================================
  Coverage   90.28%   90.28%           
=======================================
  Files          40       40           
  Lines       11704    11704           
=======================================
  Hits        10567    10567           
  Misses       1010     1010           
  Partials      127      127           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@luoliwoshang
Copy link
Member Author

@xushiwei need review

@cpunion
Copy link
Collaborator

cpunion commented Sep 9, 2025

/review

@niupilot
Copy link

niupilot bot commented Sep 9, 2025

Code Review for PR #1275

Todo List:


Review Summary

This PR updates the ESP Clang version from 19.1.2_20250820 to 19.1.2_20250905-3, which fixes a linker compatibility issue on Ubuntu 20.04 with GLIBC 2.31 (issue #1273). The changes are minimal and well-structured.

Changes Analysis

internal/crosscompile/crosscompile.go (Lines 46-47)

Good:

  • Clean version bump of ESP Clang
  • Both espClangBaseUrl and espClangVersion updated consistently
  • Version follows semantic pattern with date and build number

internal/crosscompile/fetch_test.go (Line 608)

Good:

  • Test updated to use dynamic version string via fmt.Sprintf
  • Makes test more maintainable by using espClangVersion variable
  • Prevents hardcoded version strings in tests

Code Quality Assessment

Strengths

  1. Consistency: Both URL and version variables updated together
  2. Test Maintainability: Test now dynamically references the version variable
  3. Minimal Impact: Only necessary changes made, no scope creep
  4. Clear Intent: Changes directly address the reported Ubuntu/GLIBC compatibility issue

Suggestions for Improvement

  1. Documentation: Consider adding a comment explaining the version requirement:

    // ESP Clang version - minimum 19.1.2_20250905-3 required for Ubuntu 20.04/GLIBC 2.31 compatibility
    espClangVersion = "19.1.2_20250905-3"
  2. Test Coverage: Consider adding a specific test case for the dladdr symbol issue to prevent regression

Security & Reliability

  • URL Structure: Maintains same GitHub releases pattern
  • Version Format: Follows established semantic versioning with date
  • Test Safety: Dynamic string formatting in tests is safe and improves maintainability

Issue Resolution

This PR directly addresses issue #1273 by updating to an ESP Clang version that includes fixes for the "undefined symbol: dladdr" linker error on Ubuntu 20.04. The ESP Clang prebuilt project PRs referenced in the description likely contain the actual linker compatibility fixes.

Recommendation

✅ APPROVE - This is a well-executed version bump that:

  • Fixes a specific compatibility issue
  • Maintains code consistency
  • Improves test maintainability
  • Follows established patterns
  • Has minimal risk profile

The changes are straightforward, properly tested, and directly address the reported issue.


👉 Want me to re-run this review? Just type /review in the comments! For more usage examples, visit CodeAgent GitHub Repository.

@luoliwoshang luoliwoshang changed the title internal/crosscompile:update clang with older glibc & more same size internal/crosscompile:update clang with older glibc & more small size Sep 9, 2025
@xushiwei xushiwei merged commit b72311c into goplus:main Sep 10, 2025
34 checks passed
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.

llgo v0.11.5: undefined symbol: dladdr at ubuntu20

3 participants