Skip to content

DecisionTree::dot(name, ...): skip system() shellout on iOS#2547

Merged
dellaert merged 1 commit into
borglab:developfrom
ShuangLiu1992:ios-dot-no-system
May 29, 2026
Merged

DecisionTree::dot(name, ...): skip system() shellout on iOS#2547
dellaert merged 1 commit into
borglab:developfrom
ShuangLiu1992:ios-dot-no-system

Conversation

@ShuangLiu1992
Copy link
Copy Markdown
Contributor

Problem

Building gtsam for iOS (apple-clang, iphoneos sysroot) fails on every TU that instantiates DecisionTree with:

DecisionTree-inl.h:1081:9: error: 'system' is unavailable: not available on iOS
    system(("dot -Tpdf " + name + ".dot -o " + name + ".pdf >& /dev/null")
    ^

iOS marks std::system() as __attribute__((unavailable)) in <stdlib.h>, so even a never-executed call site fails the compile.

Fix

The PDF rendering inside DecisionTree<L, Y>::dot(const std::string& name, ...) is a debug convenience layered on top of the .dot file the same function always writes. Keep the .dot emission unconditional, gate just the system(...) shellout behind !TARGET_OS_IPHONE. Callers who want a PDF on an iOS-targeted build can run dot -Tpdf <name>.dot -o <name>.pdf themselves on any host that has a shell.

    std::ofstream os((name + ".dot").c_str());
    dot(os, labelFormatter, valueFormatter, showZero);
#if defined(__APPLE__) && TARGET_OS_IPHONE
    // iOS marks std::system() unavailable, breaking the build. The PDF
    // rendering is a debug convenience over the always-emitted .dot file;
    // callers who want a PDF on iOS-targeted builds can run
    // `dot -Tpdf <name>.dot -o <name>.pdf` themselves on a host with a shell.
#else
    int result =
        system(...);
    if (result == -1)
      throw std::runtime_error("DecisionTree::dot system call failed");
#endif

<TargetConditionals.h> is added inside an #if defined(__APPLE__) guard at the top of the file so non-Apple builds don't get a missing header.

Behavior change

  • iOS builds: no longer crash to compile. Calling DecisionTree::dot(name, ...) writes the .dot file and skips the PDF rendering.
  • Every other platform (macOS, Linux, Windows, etc.): unchanged. TARGET_OS_IPHONE is 0 on macOS and undefined elsewhere; the else branch always selects, and the patch reduces to a no-op preprocessor pass.

Test

Verified by rebuilding gtsam against the iphoneos26.0 SDK with apple-clang: previously the build hit ~24 error: 'system' is unavailable across discrete/Algebraic*, Discrete*, etc.; with this patch the build completes cleanly.

Building gtsam for iOS (apple-clang, iphoneos sysroot) fails on every
DecisionTree-instantiating TU with:

    DecisionTree-inl.h:1081:9: error: 'system' is unavailable: not
        available on iOS
        system(("dot -Tpdf " + name + ".dot -o " + name + ".pdf >& /dev/null")
        ^

iOS marks std::system() as `__attribute__((unavailable))` in
<stdlib.h>, so even a never-executed call site fails the compile.

The PDF rendering is a debug convenience layered on top of the .dot
file the same function always writes — keep the .dot emission
unconditional, gate just the shellout behind !TARGET_OS_IPHONE.
Callers who want a PDF on an iOS-targeted build can run
`dot -Tpdf <name>.dot -o <name>.pdf` themselves on any host that
has a shell. Behavior on every non-iOS target is unchanged.
Copy link
Copy Markdown
Collaborator

@ProfFan ProfFan left a comment

Choose a reason for hiding this comment

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

Thank you!

@dellaert
Copy link
Copy Markdown
Member

Cool! merging.

@dellaert dellaert merged commit fc790b0 into borglab:develop May 29, 2026
40 of 44 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.

3 participants