Skip to content

Fix: Apple Silicon -- add some of the code that uses libffi#2871

Closed
mwinkel-dev wants to merge 3 commits intoMDSplus:alphafrom
mwinkel-dev:mw-2871-macos-arm64-libffi
Closed

Fix: Apple Silicon -- add some of the code that uses libffi#2871
mwinkel-dev wants to merge 3 commits intoMDSplus:alphafrom
mwinkel-dev:mw-2871-macos-arm64-libffi

Conversation

@mwinkel-dev
Copy link
Contributor

On Apple Silicon, the widely used libffi (aka Foreign Function Interface) library is used when calling variadic functions located in libraries. This PR provides some of the associated code used by the Apple Silicon port of MDSplus.

This libffi code is not used by the MDSplus versions for Linux, Windows and MacOS (Intel).

This is a partial fix for Issue #2597.

@mwinkel-dev mwinkel-dev added bug An unexpected problem or unintended behavior US Priority os/mac This is present on or relates to Mac OSX core Relates to the core libraries and scripts labels Mar 5, 2025
@mwinkel-dev
Copy link
Contributor Author

mwinkel-dev commented Mar 5, 2025

Note that MdsLib.c now includes <libroutines.h> which is why the declaration of LibCallg() has been deleted.

In the other source files, LibCallgFfi() uses the libffi library to call variadic functions in external libraries. It does the same thing as the LibCallg() function used by all other platforms. The #ifdef MACOS_ARM64 ensures that the LibCallgFfi() is only used on Apple Silicon.

A future PR will be submitted that adds the LibCallgFfi() function. The omission of the function in this PR is not a problem because the Jenkins build server is presently only building for Linux and Windows.

The TdiExtPython.c file has a hard coded path to the Python library. This path will likely change when the Jenkins build system is being configured to build MDSplus for Apple Silicon.

@mwinkel-dev mwinkel-dev force-pushed the mw-2871-macos-arm64-libffi branch from 9afeec2 to 61f6d38 Compare March 5, 2025 23:40
@mwinkel-dev
Copy link
Contributor Author

mwinkel-dev commented Mar 6, 2025

Conjecture is that this PR is A-OK, and that the Jenkins system has a flaky test-asan. Sometimes this PR builds fine, other times it fails just in test-asan.

@mwinkel-dev
Copy link
Contributor Author

Retest this please.

Copy link
Member

@WhoBrokeTheBuild WhoBrokeTheBuild left a comment

Choose a reason for hiding this comment

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

This PR cannot be submitted before the actual LibCallgFfi function is merged in.

@mwinkel-dev
Copy link
Contributor Author

mwinkel-dev commented Mar 6, 2025

Regarding the hardcoded paths in TdiExtPython.c, note that the existing code also has hardcoded paths for the Python 2.7 library. The "blame" listing shows that the hardcoded paths have been present for at least 5 years. These hardcoded paths are defaults in case the user forgets to set their PyLib environment variable before running MDSplus.

Line 128
Line 132

There are two options for dealing with this situation:

  1. Fix the hardcoded paths now, or
  2. Get the Apple Silicon code checked in and then fix the hardcoded paths.

A complicating factor on Apple Silicon is that the location of the Python library depends on whether Python 2.7 was installed by Brew, MacPorts or is part of Apple's framework. (Pretty sure that recent versions of MacOS only ship Python 3.x, they do not include Python 2.7.) To get Python 2.7 and a compatible "numpy" installed on the M2 MacBook Pro used for development, the MacPorts version worked best. Thus the port of MDSplus to Apple Silicon presently has a hardcoded path to the location of the MacPorts Python 2.7 library in the TdiExtPython.c file.

@mwinkel-dev
Copy link
Contributor Author

mwinkel-dev commented Mar 6, 2025

As discussed, Apple Silicon PRs are now being submitted / reviewed in bottom-up order. Thus this PR will be on hold until a few more PRs are submitted.

@mwinkel-dev
Copy link
Contributor Author

This PR will be revised to present a single "vertical slice" of the MDSplus code for Apple Silicon that uses the libffi library. The TdiExtPython.c change will probably be moved to a new PR.

Explanation:

Depending on the type of problem to be solved, software development uses different strategies: top-down, bottom-up, a series of vertical slices, and so forth. Each strategy is valid and has its uses.

When a program is executed, control flow is top-down. There are some changes in the Apple Silicon port of MDSplus that made it advantageous to organize the PRs in top-down order.

However, upon reflection a better approach is to present the code a vertical slice at a time. There are a few different code paths that lead to the libffi calls. For each code path (aka vertical slice), a PR will be submitted. Starting with the simplest vertical slice, then a complicated vertical slice, and ending with a problematic vertical slice.

Organizing the various changes into vertical slice PRs will take a few hours.

@mwinkel-dev
Copy link
Contributor Author

PR #2877 and PR #2878 replace this PR, therefore it is being closed.

@mwinkel-dev mwinkel-dev closed this Mar 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug An unexpected problem or unintended behavior core Relates to the core libraries and scripts os/mac This is present on or relates to Mac OSX US Priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants