Skip to content

fix: isolate embedded MD4C symbols and parser headers - #848

Closed
juliusmarminge wants to merge 1 commit into
software-mansion:mainfrom
pingdotgg:fix/embedded-md4c-isolation
Closed

juliusmarminge wants to merge 1 commit into
software-mansion:mainfrom
pingdotgg:fix/embedded-md4c-isolation

Conversation

@juliusmarminge

Copy link
Copy Markdown
Contributor

What/Why?

Fixes #846.

Linking Enriched alongside another embedded MD4C can produce a duplicate md_parse symbol. CocoaPods header maps can also resolve the iOS parsers' bare core-header includes to another dependency's headers.

  • Alias md_parse to enrm_md_parse in Enriched's embedded header. This changes the C implementation and all callers together; md_parse is the embedded C object's only exported symbol.
  • Restore the alias after fetch-md4c.sh updates the upstream sources. The vendored C implementation is unchanged.
  • Use explicit own-core relative includes in ENRMInputParser.mm and MarkdownParserBridge.mm, including the bridge's MarkdownASTNode.hpp. These resolve through the monorepo symlink and the published package's copied core tree.
  • Add a real two-copy compile/link/runtime diagnostic and run it in the existing CI lint/unit-test job. It also compiles the bridges' actual C++ includes with conflicting headers first in the search path.

Adapted from the MD4C isolation portion of the integration patch in pingdotgg/t3code #12781. That integration subsequently removed Nitro Markdown; the current T3 sole-renderer configuration does not require this fix. This PR supports generic coexistence and changes no library API or rendering behavior.

Testing

bash scripts/test-md4c-isolation.sh compiles two complete embedded MD4C copies into separate objects and links them with Enriched's real C++ parser. The second copy uses the original md_parse name. Both callback parsers and the C++ AST parser parse **bold** successfully.

Before, against the base sources:

multiple definition of `md_parse'
collect2: error: ld returned 1 exit status

After:

Two embedded MD4C copies link and parse successfully.
Bridge C++ includes resolve Enriched's own headers with the symlink layout.
Bridge C++ includes resolve Enriched's own headers with the copy layout.

The diagnostic also fails when only the symbol alias is applied, and when the original integration patch leaves the bridge's AST header ambiguous. This is compiler/runtime evidence, not a source-string assertion. Header probes compile the C++ include directives; they do not compile the Objective-C++ bridges or emulate CocoaPods header maps.

Also passed locally on Linux:

  • yarn lint, with one existing inline-style warning
  • yarn typecheck
  • yarn test --runInBand, 6 suites and 39 tests
  • yarn prepare
  • clang-format checks, Bash syntax checks, and git diff --check
  • Isolated offline sync fixture, two updates reproduce the exact current sources, and an unexpected upstream header guard fails
  • nm -g --defined-only, the isolated embedded C object exports only enrm_md_parse

Native iOS/Android builds, example apps, a real CocoaPods coexistence build, and Maestro E2E were not run for this PR head. Those remain maintainer validation gates. The existing T3 integration patch had user-reported successful iOS and Android builds, which are not claimed as verification of this revision. No screenshots apply to this build/link fix.

PR Checklist

  • Code compiles and runs on iOS
  • Code compiles and runs on Android
  • Updated documentation/README if applicable
  • Ran example app to verify changes
  • E2E tests are passing
  • Required E2E tests have been added (if applicable)

No user-visible behavior changes, so no new Maestro scenario is proposed. The focused compiler/link diagnostic is included above.

@eszlamczyk

Copy link
Copy Markdown
Collaborator

Hi @juliusmarminge Thanks for your contributions, we confirmed this is a real issue but thats not the way we want to move forward with the solution. Due to separate reasons we will be migrating to a fork of md4c, in which we will change the filenames and the functions, this will help us with the maintaining of this (and the linker with linking in specified case), we will close this pr, but keep the issue open untill we migrate the md4c

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.

Isolate embedded MD4C symbols and parser headers for coexistence

2 participants