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.

Sign up for GitHub

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

Codesigning vapor on Apple Silicon #3639

Merged
merged 24 commits into from
Jul 29, 2024
Merged

Codesigning vapor on Apple Silicon #3639

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
8e5b507
resource path working
sgpearse Jul 23, 2024
fea29ce
resource path working on local builds
sgpearse Jul 23, 2024
6059bcd
cleanup and linux test
sgpearse Jul 23, 2024
56e16c4
cleanup
sgpearse Jul 23, 2024
04d43bb
build test
sgpearse Jul 23, 2024
8b1fed9
build test
sgpearse Jul 23, 2024
7e6a00a
build test
sgpearse Jul 23, 2024
a946e48
build test
sgpearse Jul 23, 2024
c4139c5
build test
sgpearse Jul 23, 2024
6d1c381
build test
sgpearse Jul 24, 2024
4e03696
macos test
sgpearse Jul 24, 2024
d1b2d1c
codesigning validated
sgpearse Jul 25, 2024
90d4736
test
sgpearse Jul 25, 2024
65d1255
final test 1
sgpearse Jul 25, 2024
10effd9
final test 2
sgpearse Jul 25, 2024
46ed3b6
final test 3
sgpearse Jul 25, 2024
11999a9
final test 4
sgpearse Jul 25, 2024
e5d91aa
final test 5
sgpearse Jul 25, 2024
0173b8a
final test 6
sgpearse Jul 25, 2024
a61edc4
final test 7
sgpearse Jul 25, 2024
37b1ba3
final test 8
sgpearse Jul 25, 2024
1ae4ab7
final test 9
sgpearse Jul 25, 2024
b6e2604
final commit
sgpearse Jul 25, 2024
c808979
turn ospray off
sgpearse Jul 25, 2024
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
246 changes: 73 additions & 173 deletions .circleci/config.yml
View file
Open in desktop

Large diffs are not rendered by default.

64 changes: 44 additions & 20 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,11 @@ else()

list (APPEND CMAKE_PREFIX_PATH ${THIRD_PARTY_LIB_DIR})
list (APPEND CMAKE_PREFIX_PATH ${THIRD_PARTY_DIR})
list (APPEND CMAKE_PREFIX_PATH ${THIRD_PARTY_DIR}/HDF_Group/HDF5/1.12.2/lib)
if ("${CMAKE_OSX_ARCHITECTURES}" STREQUAL "arm64")
list (APPEND CMAKE_PREFIX_PATH ${THIRD_PARTY_DIR}/h5/lib)
else()
list (APPEND CMAKE_PREFIX_PATH ${THIRD_PARTY_DIR}/HDF_Group/HDF5/1.12.2/lib)
endif()

if (APPLE)
set(CMAKE_MODULE_PATH /opt/local/lib/libomp)
Expand Down Expand Up @@ -210,23 +214,33 @@ message("Library EXPAT = ${EXPAT}")
function(FIND_BUNDLED_PYTHON)
# FindPython supports Python_ROOT_DIR however vapor's bundled python distribution
# does not conform to its requirements so this manually configures the results
message("Using bundled python")
message("Using bundled python ${PYTHONDIR} ${PYTHONPATH}")
set(Python_VERSION "${PYTHONVERSION}")
set(Python_NumPy_INCLUDE_DIRS "${NUMPY_INCLUDE_DIR}")
unset(Python_LIBRARIES) # This is required for find_library to work in certain cases
if ("${CMAKE_OSX_ARCHITECTURES}" STREQUAL "arm64")
set(PYTHON_LIB_DIR "${PYTHONDIR}/lib")
else()
set(PYTHON_LIB_DIR "${PYTHONPATH}")
endif()
find_library(
Python_LIBRARIES
NAMES python${PYTHONVERSION} python${PYTHONVERSION}m
PATHS ${THIRD_PARTY_LIB_DIR} ${PYTHONPATH}
NAMES python${PYTHONVERSION} python${PYTHONVERSION}m
PATHS ${THIRD_PARTY_LIB_DIR} ${PYTHON_LIB_DIR}
NO_DEFAULT_PATH
)

if (WIN32)
set(Python_SITELIB "${PYTHONPATH}/Lib/site-packages")
set(Python_INCLUDE_DIRS "${THIRD_PARTY_DIR}/Python${PYTHONVERSION}/include")
else()
set(Python_SITELIB "${PYTHONPATH}/site-packages")
if (NOT DEFINED Python_INCLUDE_DIRS)
set(Python_INCLUDE_DIRS "${THIRD_PARTY_INC_DIR}/python${PYTHONVERSION}")
if ("${CMAKE_OSX_ARCHITECTURES}" STREQUAL "arm64")
set(Python_INCLUDE_DIRS "${PYTHONDIR}/include/python${PYTHONVERSION}")
else()
set(Python_INCLUDE_DIRS "${THIRD_PARTY_INC_DIR}/python${PYTHONVERSION}")
endif()
endif()
endif()

Expand Down Expand Up @@ -301,9 +315,12 @@ elseif (APPLE)
set (CMAKE_INSTALL_PREFIX /Applications)
set (INSTALL_BIN_DIR ./vapor.app/Contents/MacOS)
set (INSTALL_SHARE_DIR ./vapor.app/Contents/share)
#set (INSTALL_LIB_DIR ./vapor.app/Contents/lib)
set (INSTALL_LIB_DIR ./vapor.app/Contents/Frameworks)
set (INSTALL_INCLUDE_DIR ./vapor.app/Contents/include/vapor)
if ("${CMAKE_OSX_ARCHITECTURES}" STREQUAL "arm64")
set (INSTALL_LIB_DIR ./vapor.app/Contents/Frameworks)
else ()
set (INSTALL_LIB_DIR ./vapor.app/Contents/lib)
endif()
else ()
set (INSTALL_BIN_DIR bin)
set (INSTALL_LIB_DIR lib)
Expand All @@ -312,10 +329,11 @@ elseif (APPLE)
endif ()

if (BUILD_PYTHON)
set (CMAKE_INSTALL_RPATH "@loader_path")
else ()
#set (CMAKE_INSTALL_RPATH "@executable_path/../lib")
set (CMAKE_INSTALL_RPATH "@executable_path/../Frameworks")
set (CMAKE_INSTALL_RPATH "@loader_path")
elseif ("${CMAKE_OSX_ARCHITECTURES}" STREQUAL "arm64")
set (CMAKE_INSTALL_RPATH "@executable_path/../Frameworks;@executable_path/../Resources")
Copy link
Collaborator

Choose a reason for hiding this comment

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

RPATH is for finding libraries/frameworks which for an AppBundle should exist inside the Frameworks directory. Is there a reason we also need it to link to the Resources directory?

From the macOS docs:

Frameworks: Contains any private shared libraries and frameworks used by the executable
Resources: data files that live outside your application’s executable file. Resources typically consist of things like images, icons, sounds, nib files, strings files, configuration files, and data files

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, it's a codesigning requirement. Frameworks cannot contain non-binary files like .py modules, so Python needs to reside in Resources.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just a followup with a reference. This is also how Blender distributes Python.

Store Python, Perl, shell, and other script files and other non-Mach-O executables in your app's Contents/Resources directory. While it's possible to sign such executables and store them in Contents/MacOS, this is not recommended.

https://developer.apple.com/library/archive/technotes/tn2206/_index.html#//apple_ref/doc/uid/DTS40007919-CH1-TNTAG13

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see the issue and I am fine keeping it this way.

To respond to your followup:

Store Python, Perl, shell, and other script files and other non-Mach-O executables in your app's Contents/Resources directory. While it's possible to sign such executables and store them in Contents/MacOS, this is not recommended.

Blender's macOS app does not follow the specification at all.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, thanks, but for the record:

Blender's macOS app does not follow the specification at all.

Blender puts all of its libraries in the Resources directory. Could you elaborate if I'm mistaken?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Correct, which directly contradicts the specification.

else()
set (CMAKE_INSTALL_RPATH "@executable_path/../lib")
endif ()

if (DIST_INSTALLER AND USE_OMP)
Expand Down Expand Up @@ -428,18 +446,11 @@ if (APPLE)
set (CPACK_BINARY_DRAGNDROP ON)
if (DIST_INSTALLER AND GENERATE_FULL_INSTALLER)
file (GLOB INSTALL_LIBS ${THIRD_PARTY_LIB_DIR}/*.dylib)
#file (GLOB INSTALL_FRAMEWORKS ${THIRD_PARTY_LIB_DIR}/*.framework)
file (GLOB INSTALL_GUI_FRAMEWORKS ${PYTHONPATH})
install (
FILES ${INSTALL_LIBS}
DESTINATION ${INSTALL_LIB_DIR}
COMPONENT Dependencies
)
#install (
# DIRECTORY ${INSTALL_FRAMEWORKS}
# DESTINATION ${INSTALL_LIB_DIR}
# COMPONENT Dependencies
# )

set (FRAMEWORKS Core OpenGL Widgets Gui DBus Network PrintSupport)
foreach(item IN LISTS FRAMEWORKS)
Expand All @@ -463,9 +474,22 @@ if (APPLE)
endif ()

if (NOT BUILD_PYTHON)
if ("${CMAKE_OSX_ARCHITECTURES}" STREQUAL "arm64")
file (GLOB INSTALL_GUI_FRAMEWORKS ${PYTHONDIR})
set (PYTHON_DESTINATION "${INSTALL_LIB_DIR}/../Resources")
else()
file (GLOB INSTALL_GUI_FRAMEWORKS ${PYTHONPATH})
set (PYTHON_DESTINATION "${INSTALL_LIB_DIR}")
endif()
install (
DIRECTORY ${INSTALL_GUI_FRAMEWORKS}
DESTINATION ${INSTALL_LIB_DIR}
StasJ marked this conversation as resolved.
Show resolved Hide resolved
DIRECTORY ${INSTALL_GUI_FRAMEWORKS}
DESTINATION ${PYTHON_DESTINATION}
COMPONENT Dependencies
PATTERN "bin" EXCLUDE
)
install (
FILES ${Python_LIBRARIES}
DESTINATION ${PYTHON_DESTINATION}
COMPONENT Dependencies
)
file (GLOB COCOA_LIBS ${THIRD_PARTY_DIR}/plugins/platforms/libqcocoa.dylib)
Expand Down
61 changes: 61 additions & 0 deletions buildutils/codesignMacOS.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
set +e

installDir="/Applications"
dmgName="VAPOR3-3.9.3-AppleSilicon.dmg"
dmgDir="tmp"

#for file in ${installDir}/vapor.app/Contents/Frameworks/*; do
# codesign --sign "Developer ID Application: University Corporation for Atmospheric Research (DQ4ZFL4KLF)" --force --deep --timestamp --verbose $file
#done

# Deploy Qt with macdeployqt
# N.B. Apple's notorization will give an "bundle format is ambiguous (could be app or framework)" if Qt's frameworks are missing symlinks
# So, if copying the third party libraries use 'cp -a /old/libs /new/libs' and not 'cp -r /old/libs /new/libs' (-a and not -r)
/usr/local/VAPOR-Deps/current/bin/macdeployqt ${installDir}/vapor.app -timestamp -sign-for-notarization="Developer ID Application: University Corporation for Atmospheric Research (DQ4ZFL4KLF)"

# Codesign libraries and frameworks
find ${installDir}/vapor.app/Contents/Frameworks -type f -exec file {} + | grep -E 'Mach-O .* shared library' | cut -d: -f1 | while read -r file; do
codesign --sign "Developer ID Application: University Corporation for Atmospheric Research (DQ4ZFL4KLF)" --timestamp --force --deep --verbose --options runtime "$file"
done

# Codesign plugins and python
directories=(
"${installDir}/vapor.app/Contents/share/plugins/"
"${installDir}/vapor.app/Contents/Resources/"
"${installDir}/vapor.app/Contents/Resources/python/lib/"
"${installDir}/vapor.app/Contents/Resources/python/lib/python3.9/site-packages/numpy/.dylibs"
"${installDir}/vapor.app/Contents/Resources/python/lib/python3.9/site-packages/PIL/.dylibs"
"${installDir}/vapor.app/Contents/Resources/python/lib/python3.9/site-packages/scipy/.dylibs"
)
for dir in "${directories[@]}"; do
find "$dir" -type f \( -name "*.so" -o -name "*.dylib" \) | while read -r file; do
codesign --sign "Developer ID Application: University Corporation for Atmospheric Research (DQ4ZFL4KLF)" --timestamp --force --deep --verbose $file
done
done

# Codesign additional MacOS plugins
codesign --sign "Developer ID Application: University Corporation for Atmospheric Research (DQ4ZFL4KLF)" --force --deep --verbose ${installDir}/vapor.app/Contents/MacOS/platforms/libqcocoa.dylib
codesign --sign "Developer ID Application: University Corporation for Atmospheric Research (DQ4ZFL4KLF)" --force --deep --verbose ${installDir}/vapor.app/Contents/MacOS/styles/libqmacstyle.dylib

# Codesign all vapor executables with timestamp and hardended runtime
for file in ${installDir}/vapor.app/Contents/MacOS/*; do
codesign --sign "Developer ID Application: University Corporation for Atmospheric Research (DQ4ZFL4KLF)" --force --deep --timestamp --verbose --options runtime $file
done

# Create .dmg that contains symlink to /Applications
mkdir -p "${dmgDir}"
cp -R "${installDir}/vapor.app" "${dmgDir}"
ln -s /Applications "${dmgDir}/Applications"
hdiutil create -volname "vapor" -srcfolder ${dmgDir} -ov -format UDZO "${installDir}/${dmgName}"
rm -rf "${dmgDir}"

# Sign and notarize the .dmg
codesign --force --verify --verbose --sign "Developer ID Application: University Corporation for Atmospheric Research (DQ4ZFL4KLF)" ${installDir}/${dmgName}
xcrun -v notarytool submit ${installDir}/${dmgName} --keychain-profile "testApp-password" --wait

# Staple the notarized .dmg
xcrun stapler staple ${installDir}/${dmgName}
xcrun stapler validate ${installDir}/${dmgName}

# Fetch the notorization log
# xcrun notarytool log --keychain-profile "testApp-password" <hash>
4 changes: 4 additions & 0 deletions lib/common/GetAppPath.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,10 @@ std::string Wasp::GetAppPath(const string &app, const string &resource, const ve
path.append("share");
} else if (myResource.compare("lib") == 0) {
path.append("lib");
} else if (myResource.compare("Frameworks") == 0) {
path.append("Frameworks");
} else if (myResource.compare("Resources") == 0) {
path.append("Resources");
} else if (myResource.compare("home") == 0) {
path.erase(path.size() - 1, 1);
} else { // must be plugins
Expand Down
20 changes: 13 additions & 7 deletions lib/common/ResourcePath.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,10 @@ std::string Wasp::GetResourcePath(const std::string &name)

std::string Wasp::GetSharePath(const std::string &name) { return GetResourcePath("share/" + name); }

#ifdef WIN32
#if defined(WIN32)
#define PYTHON_INSTALLED_PATH ("python" + string(PYTHON_VERSION))
#elif defined(__aarch64__)
#define PYTHON_INSTALLED_PATH ("Resources/python")
#else
#define PYTHON_INSTALLED_PATH ("lib/python" + string(PYTHON_VERSION))
#endif
Expand All @@ -119,12 +121,16 @@ std::string Wasp::GetPythonDir()
#ifdef WIN32
return GetPythonPath();
#endif

string path = GetResourcePath("");

string exists = FileUtils::JoinPaths({path, PYTHON_INSTALLED_PATH});

if (!FileUtils::Exists(FileUtils::JoinPaths({path, PYTHON_INSTALLED_PATH}))) path = string(PYTHON_DIR);
string path = string(PYTHON_DIR); // Try our third-party-library directory first
Copy link
Collaborator

Choose a reason for hiding this comment

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

We want to use the installed python dir first, and only have the developer libs as a fallback.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a fairly obscure part of our codebase. I'd recommend revisiting it after 3.9.3 but we can postpone the release if it's that important.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure what you mean by obscure, this determines the version of python that is used by Vapor so it is rather critical. With this change it will be impossible for a developer to test an installed version of Vapor without first removing their third party library installation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I understand and totally see you point. IMO, I don't think it's critical to fix now, but it's critical to fix soon because installers are not often tested on dev computers. This is why we do platform tests on clean (non-dev) systems.

If this solution didn't work as in this PR, I would agree to modify it as you've suggested.

This file is obscure because:

  1. It heavily relies on macro'd functions that cannot be plugged into a debugger like lldb. GetResourcePath() can't be debugged without adding print statements to see what happens
  2. PYTHON_INSTALLED_PATH is also macro'd
  3. CMake code injections are furthermore specified from lib/common/CMakeConfig.cpp.in. This is as hard to debug as macro's, but the changes are coming from yet another compile time modification
  4. The #ifdef WIN32 macro for GetPythonPath() should be in-lined with GetPythonDir(). What's the difference between a PythonPath and a PythonDir?Apparently it's windows is dir, unix is path? Path and Dir are synonyms so this makes no sense.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I use the installers to test on my dev machine frequently when I am unsure if an issue is related to new code or my dev environment. This invalidates such a test. It also creates the situation where if someone has a different incompatible version of the third part libs installed (older or newer) vapor may try to load that version and crash. A few months down the line we may forget about this behavior and have to figure out why vapor is crashing. Since this is only going to affect developers not end users I'm fine with not holding up the release over this. Also, read obscure as not important or well known.

if (!FileUtils::Exists(path)) {
path = GetResourcePath("");
string exists = FileUtils::JoinPaths({path, PYTHON_INSTALLED_PATH});
if (!exists.empty()) {
#if defined(__aarch64__)
path = exists; // If the third-party-library directory doesn't exist, use the python installed path. Otherwise, use the root.
#endif
}
}
return path;
}

Expand Down
12 changes: 12 additions & 0 deletions lib/render/OSPRay.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@ using namespace VOSP;
#ifdef BUILD_OSPRAY
static int _initialized = false;

#if defined(__aarch64__)
void ospErrorCallback(void* userData, OSPError error, const char *msg)
#else
void ospErrorCallback(OSPError error, const char *msg)
#endif
{
fprintf(stderr, "OSP error ");
switch (error) {
Expand Down Expand Up @@ -44,7 +48,11 @@ int VOSP::Initialize(int *argc, char **argv)
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
// If this is not set, OSPRay will crash upon shutdown
#if defined(__aarch64__)
ospDeviceSetErrorCallback(ospGetCurrentDevice(), ospErrorCallback, nullptr);
#else
ospDeviceSetErrorFunc(ospGetCurrentDevice(), ospErrorCallback);
#endif
#pragma GCC diagnostic pop
_initialized = true;
return 0;
Expand Down Expand Up @@ -184,7 +192,11 @@ OSPGeometricModel Test::LoadTriangle(glm::vec3 scale, const std::string &rendere

ospCommit(mesh);

#if defined(__aarch64__)
OSPMaterial mat = ospNewMaterial(rendererType.c_str());
#else
OSPMaterial mat = ospNewMaterial(rendererType.c_str(), "obj");
#endif
ospCommit(mat);

// put the mesh into a model
Expand Down
57 changes: 28 additions & 29 deletions scripts/build3rdParty.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,7 @@ pythonVapor() {
#LDFLAGS="-L$installDir/lib -L$(brew --prefix zlib)/lib -I$(brew --prefix openssl)/lib" \
#CPPFLAGS="-I$installDir/include -I$(brew --prefix zlib)/include -I$(brew --prefix openssl)/include" \
if [ "$OS" = "macOSx86" ] || [ "$OS" = "M1" ]; then
args+=(--libdir=$installDir/Resources)
args+=(--with-openssl=$(brew --prefix openssl@1.1))
args+=(--with-tcltk-libs="$(pkg-config --libs tcl tk)")
args+=(--with-tcltk-includes="$(pkg-config --cflags tcl tk)")
Expand Down Expand Up @@ -768,36 +769,34 @@ elif [ "$OS" == "suse" ]; then
susePrerequisites
fi

openssl
libpng
jpeg
tiff
sqlite
ssh
curl
proj
geotiff

# openssl
#fi
zlib
#openssl
#libpng
#jpeg
#tiff
#sqlite
#ssh
#curl
#proj
#geotiff
##fi
#zlib
pythonVapor
assimp
szip
hdf5
netcdf
expat
udunits
freetype
if [ "$OS" == "Ubuntu" ] ; then
xinerama
fi
ospray
glm
gte
images
qt
#assimp
#szip
#hdf5
#netcdf
#expat
#udunits
#freetype
#if [ "$OS" == "Ubuntu" ] ; then
# xinerama
#fi
#ospray
#glm
#gte
#images
#qt
if [ "$OS" == "macOSx86" ] || [ "$OS" == "M1" ]; then
add_rpath
fi
renameAndCompress
#renameAndCompress
9 changes: 7 additions & 2 deletions site_files/site.NCAR
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,13 @@ if (UNIX)
if (NOT DEFINED PYTHONVERSION)
set (PYTHONVERSION 3.9)
endif()
set (PYTHONDIR ${THIRD_PARTY_DIR})
set (PYTHONPATH ${THIRD_PARTY_LIB_DIR}/python${PYTHONVERSION})
if ("${CMAKE_OSX_ARCHITECTURES}" STREQUAL "arm64")
set (PYTHONDIR ${THIRD_PARTY_DIR}/Resources/python)
set (PYTHONPATH ${PYTHONDIR}/lib/python${PYTHONVERSION})
else ()
set (PYTHONDIR ${THIRD_PARTY_DIR})
set (PYTHONPATH ${THIRD_PARTY_LIB_DIR}/python${PYTHONVERSION})
endif()
set (EXTRA_LIBS_SEARCH ${EXTRA_LIBS_SEARCH} dbus)
elseif (WIN32)
set (PYTHONVERSION 36)
Expand Down