-
Notifications
You must be signed in to change notification settings - Fork 46
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
base: master
Are you sure you want to change the base?
macOS build fix #16
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 |
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
||
if [ "x$1" = "xsigrok-cli" ]; then | ||
APPNAME="sigrok-cli" | ||
APPNAME_BINARY="sigrok-cli" | ||
|
@@ -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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. indentation is all over the place There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. | ||
|
||
|
@@ -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 | ||
|
@@ -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 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
||
|
@@ -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" | ||
|
@@ -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 | ||
|
||
|
@@ -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 | ||
|
@@ -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 | ||
|
@@ -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 ../.. | ||
|
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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:
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 settingAPPLE_SILICON
ton
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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,