Repository navigation
fix: isolate embedded MD4C symbols and parser headers - #848
Closed
juliusmarminge wants to merge 1 commit into
Closed
juliusmarminge wants to merge 1 commit into
juliusmarminge wants to merge 1 commit into
Conversation
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 |
4 of 6 tasks
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.
What/Why?
Fixes #846.
Linking Enriched alongside another embedded MD4C can produce a duplicate
md_parsesymbol. CocoaPods header maps can also resolve the iOS parsers' bare core-header includes to another dependency's headers.md_parsetoenrm_md_parsein Enriched's embedded header. This changes the C implementation and all callers together;md_parseis the embedded C object's only exported symbol.fetch-md4c.shupdates the upstream sources. The vendored C implementation is unchanged.ENRMInputParser.mmandMarkdownParserBridge.mm, including the bridge'sMarkdownASTNode.hpp. These resolve through the monorepo symlink and the published package's copied core tree.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.shcompiles two complete embedded MD4C copies into separate objects and links them with Enriched's real C++ parser. The second copy uses the originalmd_parsename. Both callback parsers and the C++ AST parser parse**bold**successfully.Before, against the base sources:
After:
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 warningyarn typecheckyarn test --runInBand, 6 suites and 39 testsyarn preparegit diff --checknm -g --defined-only, the isolated embedded C object exports onlyenrm_md_parseNative 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
No user-visible behavior changes, so no new Maestro scenario is proposed. The focused compiler/link diagnostic is included above.