Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

macOS build fix #16

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
build/
cross-compile/macosx/build*
firmware/kingst-la/*.bitstream
firmware/kingst-la/*.fw
27 changes: 0 additions & 27 deletions cross-compile/macosx/contrib/pulseview

This file was deleted.

27 changes: 0 additions & 27 deletions cross-compile/macosx/contrib/sigrok-cli

This file was deleted.

84 changes: 69 additions & 15 deletions cross-compile/macosx/create_dmg
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ set -e
# The path where the installed sigrok libraries/binaries are located.
PREFIX=$HOME/sr_macosx

# Build for Apple Silicon (y/n)
APPLE_SILICON=y
Copy link
Contributor

@fenugrec fenugrec Nov 19, 2023

Choose a reason for hiding this comment

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

some more context in here would be useful about what "apple silicon" refers to, i.e. what generation / years / cpu type / OS version / whatever, for future refernce by non-apple-specialists or casual devs

Copy link
Author

@v-kiniv v-kiniv Nov 19, 2023

Choose a reason for hiding this comment

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

I tested this PR on macOS 14.1.1 / M1 CPU.

As I stated in description:

APPLE_SILICON option is figurative, I think most fixes are relevant to Intel Mac running latest macOS, so read it as LATEST_MACOS.

I believe it's relevant for macOS 11 and beyond. But since there's no mention in documentation what versions of macOS currently supported by sigrok-util/PulseView it's hard for me to delineate what versions is APPLE_SILICON=y or =n. That's why I added this changes as an option, and not permanent changes, so you can revert to original behaviour by setting APPLE_SILICON to n.

Copy link
Contributor

Choose a reason for hiding this comment

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

I tested this PR on macOS 14.1.1 / M1 CPU.

I believe it's relevant for macOS 11 and beyond.

That's a good start, and that kind of info should definitely not just be deep in here in a comment thread but at least at the top in the PR submission, commit message etc. Otherwise in 2-3 months when another macos user comes back with issues / fixes, then nobody knows what this apply to. This kind of information gets lost easily which is exactly why , as you say,

there's no mention in documentation what versions of macOS currently supported by sigrok-util/PulseView


if [ "x$1" = "xsigrok-cli" ]; then
APPNAME="sigrok-cli"
APPNAME_BINARY="sigrok-cli"
Expand All @@ -34,20 +37,34 @@ fi
# The path where to download files to and where to build packages.
BUILDDIR=./build_app_$APPNAME_BINARY

# We use Qt 5.5 in order to remain compatible with more versions of Mac OS X.
[email protected]
if [[ $APPLE_SILICON = y ]]; then
# Use Qt6 if building for Apple Silicon
QTVER=qt@6
else
# We use Qt 5.5 in order to remain compatible with more versions of Mac OS X.
Copy link
Contributor

Choose a reason for hiding this comment

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

indentation is all over the place

Copy link
Author

Choose a reason for hiding this comment

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

You are reviewing changes from the first commit only, this was fixed in the second commit.

Copy link
Contributor

Choose a reason for hiding this comment

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

Then modify the first commit to be correct and not require a 'fix' ?

[email protected]
fi

# Path to Qt5 binaries.
QTBINDIR=`brew list $QTVER | grep bin | head -n 1 | xargs dirname`
QTTRANSLATIONSDIR=`brew --prefix $QTVER`/translations
if [[ $APPLE_SILICON = y ]]; then
QTTRANSLATIONSDIR=`brew --prefix $QTVER`/share/qt/translations
else
TRANSLATIONSDIR=`brew --prefix $QTVER`/translations
fi

# Path to boost libraries.
BOOSTLIBDIR=`brew list boost | grep libboost_system | grep -v cmake | head -n 1 | xargs dirname`

# Path to Python 3 framework.
PYTHONFRAMEWORKDIR=`brew list python3 | grep libpython | head -n 1 | xargs dirname`/../../../..
# Extract path and version of python3 framework from libsigrokdecode using otool
PYTHONFRAMEWORKDIR=$(otool -L $PREFIX/lib/libsigrokdecode.*.dylib | grep python | head -n 1)
PYTHONFRAMEWORKDIR=${PYTHONFRAMEWORKDIR%"/Python.framework"*}
PYTHONFRAMEWORKDIR=$(readlink -f $PYTHONFRAMEWORKDIR)/
PYVER=${PYTHONFRAMEWORKDIR#*@}
PYVER=${PYVER%%"/"*}

PYVER="3.7"
# Path to dbus
DBUSLIBDIR=`brew list dbus | grep bin | head -n 1 | xargs dirname`/../lib

# You usually don't need to change anything below this line.

Expand Down Expand Up @@ -85,6 +102,15 @@ if [ "x$APPNAME_BINARY" = "xpulseview" ]; then
mkdir -p $CONTENTSDIR/translations
cp $QTTRANSLATIONSDIR/qt_*.qm $CONTENTSDIR/translations
cp $QTTRANSLATIONSDIR/qtbase_*.qm $CONTENTSDIR/translations

if [[ $APPLE_SILICON = y ]]; then
# Copy QtDBus, which should be but is not actually copied by macdeployqt
cp -R $QTBINDIR/../lib/QtDBus.framework $FRAMEWORKSDIR
rm -rf $FRAMEWORKSDIR/QtDBus.framework/Headers
# Copy dbus
cp $DBUSLIBDIR/*.dylib $FRAMEWORKSDIR
chmod 644 $FRAMEWORKSDIR/libdbus*.dylib
fi
fi

$QTBINDIR/macdeployqt $APPNAME.app
Expand All @@ -109,18 +135,46 @@ rm -rf $PYDIR/lib/python$PYVER/__pycache__
rm -rf $PYDIR/lib/python$PYVER/**/__pycache__
rm -rf $PYDIR/lib/python$PYVER/**/**/__pycache__
rm -rf $PYDIR/Resources
install_name_tool -change \
/usr/local/opt/python/Frameworks/Python.framework/Versions/$PYVER/Python \
@executable_path/../Frameworks/Python.framework/Versions/$PYVER/Python \
$FRAMEWORKSDIR/libsigrokdecode.*.dylib
install_name_tool -change \
/usr/local/opt/python/Frameworks/Python.framework/Versions/$PYVER/Python \
@executable_path/../Frameworks/Python.framework/Versions/$PYVER/Python \
$PREFIX/lib/libirmp.*.dylib

PYFRAMEWORKSDIR_SRC=/usr/local/opt/python/Frameworks/Python.framework/Versions/$PYVER/Python
if [[ $APPLE_SILICON = y ]]; then
PYFRAMEWORKSDIR_SRC=/opt/homebrew/opt/python@$PYVER/Frameworks/Python.framework/Versions/$PYVER/Python
fi
PYFRAMEWORKSDIR_DST=@executable_path/../Frameworks/Python.framework/Versions/$PYVER/Python
install_name_tool -change $PYFRAMEWORKSDIR_SRC $PYFRAMEWORKSDIR_DST $FRAMEWORKSDIR/libsigrokdecode.*.dylib
install_name_tool -change $PYFRAMEWORKSDIR_SRC $PYFRAMEWORKSDIR_DST $PREFIX/lib/libirmp.*.dylib

if [[ $APPLE_SILICON = y ]]; then
# Sign all binaries recursively
function sign_all {
for f in $1/*; do
file_type_str=$(file -Ih $f)
mime=${file_type_str#*: }
if [[ $mime = "inode/directory;"* ]]; then
sign_all $f
elif [[ $mime = "application/x-mach-binary;"* ]]; then
codesign --force -s - $f &> /dev/null || :
fi
done
}
sign_all $MACOSDIR
sign_all $FRAMEWORKSDIR
sign_all $CONTENTSDIR/PlugIns
fi

# Add wrapper (sets PYTHONHOME/SIGROK_FIRMWARE_DIR/SIGROKDECODE_DIR).
mv $MACOSDIR/$APPNAME_BINARY $MACOSDIR/$APPNAME_BINARY.real
cp ../contrib/$APPNAME_BINARY $MACOSDIR
cat <<EOT >> $MACOSDIR/$APPNAME_BINARY
#!/bin/sh

DIR="\$(dirname "\$0")"
cd "\$DIR"
export DYLD_LIBRARY_PATH="../Frameworks"
export PYTHONHOME="../Frameworks/Python.framework/Versions/$PYVER"
export SIGROK_FIRMWARE_DIR="../share/sigrok-firmware"
export SIGROKDECODE_DIR="../share/libsigrokdecode/decoders"
exec ./$APPNAME_BINARY.real "\$@"
EOT
chmod 755 $MACOSDIR/$APPNAME_BINARY

cp ../contrib/Info.plist_$APPNAME_BINARY $CONTENTSDIR/Info.plist
Expand Down
46 changes: 39 additions & 7 deletions cross-compile/macosx/sigrok-native-macosx
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,13 @@ set -e
# The path where the compiled packages will be installed.
PREFIX=$HOME/sr_macosx

# Run tests (y/n)
ENABLE_TESTS=n

# Build for Apple Silicon (y/n)
# Tests is currently failing on Apple Silicon, disable tests by setting ENABLE_TESTS=n
APPLE_SILICON=y

# The path where to download files to and where to build packages.
BUILDDIR=./build

Expand All @@ -44,8 +51,13 @@ PARALLEL="-j "`sysctl -n hw.ncpu`
export CC=gcc
export CXX=g++

# We use Qt 5.5 in order to remain compatible with more versions of Mac OS X.
[email protected]
if [[ $APPLE_SILICON = y ]]; then
# Use Qt6 if building for Apple Silicon
QTVER=qt@6
else
# We use Qt 5.5 in order to remain compatible with more versions of Mac OS X.
[email protected]
fi

# Path to Qt5 binaries (needed for cmake to find the Qt5 libs).
export PATH="$(brew --prefix $QTVER)/bin:$PATH"
Expand All @@ -54,10 +66,24 @@ export PATH="$(brew --prefix $QTVER)/bin:$PATH"

# -----------------------------------------------------------------------------

if [[ $APPLE_SILICON = y ]]; then
qt5_found=$(brew list --versions qt@5 || :)
if [[ $qt5_found ]]; then
echo "\033[0;31mWarning: PulseView build will most likely fail, because Qt5 is installed and will be picked by CMake over Qt6. Uninstall Qt5 or modify PulseView CMake.\033[0m"
fi
if [[ $ENABLE_TESTS = y ]]; then
echo "\033[0;31mWarning: Tests will most likely fail on Apple Silicon, disable tests by setting ENABLE_TESTS=n\033[0m"
fi
fi

# PKG_CONFIG_PATH will need to point to pkg-config files of Homebrew's
# keg-only formulae.
FORMULAS=("libffi" "python@3")
if [[ $APPLE_SILICON = n ]]; then
FORMULAS+=("python@2" "$QTVER")
fi
P="$PREFIX/lib/pkgconfig"
for FORMULA in libffi python@2 python@3 "$QTVER"; do
for FORMULA in ${FORMULAS[@]}; do
P="$P:$(brew --prefix "$FORMULA")/lib/pkgconfig"
done

Expand Down Expand Up @@ -96,7 +122,9 @@ cd build
PKG_CONFIG_PATH=$P ../configure $C
$SB make $PARALLEL $V
PYTHONPATH=$PYPATH $SB make install $V
$SB make check $V
if [[ $ENABLE_TESTS = y ]]; then
$SB make check $V
fi
cd ../..

# libsigrokdecode
Expand All @@ -108,7 +136,9 @@ cd build
PKG_CONFIG_PATH=$P ../configure $C
$SB make $PARALLEL $V
make install $V
$SB make check $V
if [[ $ENABLE_TESTS = y ]]; then
$SB make check $V
fi
cd ../..

# sigrok-firmware
Expand Down Expand Up @@ -150,9 +180,11 @@ $GIT_CLONE $REPO_BASE/pulseview
cd pulseview
mkdir build
cd build
PKG_CONFIG_PATH=$P $SB cmake -DCMAKE_INSTALL_PREFIX:PATH=$PREFIX -DDISABLE_WERROR=y -DENABLE_TESTS=y ..
PKG_CONFIG_PATH=$P $SB cmake -DCMAKE_INSTALL_PREFIX:PATH=$PREFIX -DDISABLE_WERROR=y -DENABLE_TESTS=$ENABLE_TESTS ..
$SB make $PARALLEL $V
make install $V
$SB make test $V
if [[ $ENABLE_TESTS = y ]]; then
$SB make test $V
fi
cd ../..