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

Translocation fix #273

Merged
merged 24 commits into from
Mar 20, 2024
Merged

Translocation fix #273

merged 24 commits into from
Mar 20, 2024

Conversation

firthm01
Copy link
Contributor

@firthm01 firthm01 commented Mar 14, 2024

Some mac machines seem to use translocation on the setup application bundle even though the application and the disk image itself is signed. This is not expected behaviour going by Apple Developer documentation - a signed and notorised disk image containing signed binaries should not need any translocation. When translocation happens, the setup application can not locate the setup files as the application has been moved to a random directory. Perhaps a bug? Only seems to affect the odd machine.

In any case, we need a workaround. This fix places all of the setup files in the Resources folder of the setup application. This way, even if translocation occurs, setup still has access to all of the setup files it needs.

  • If NOT running on CI, CMake will copy resources in to setup bundle as post build step - this allows for local testing.
  • If running on CI, the resources will be copied in to the bundle as part of packaging by setting the install directory for the other targets to Setup EAR Production Suite.app/Contents/Resources

Other bits to support it;

  • CMake scripts keep track of all of the EPS VST targets as they're added. This is pushed up to root scope (hence lots of PARENT_SCOPE sets to push it up through the chain.) Less chance of forgetting any new plugin during copy stage this way.
  • Setup application code changed to look within own Resources folder for plugins/extension/templates/tools etc when installing
  • CMake version parsing code updated to not look for a hyphen separator for final section of a version. This allows us to have "1.1.0b".
  • Updated changelog for the "b" release
  • Added setup icon (otherwise macos seems to just choose a random thing from inside it's Resources folder)
  • Codesigning script updated

@firthm01 firthm01 requested a review from rsjbailey March 14, 2024 15:35
@firthm01 firthm01 self-assigned this Mar 14, 2024
@firthm01 firthm01 marked this pull request as ready for review March 18, 2024 10:02
Copy link
Contributor

@rsjbailey rsjbailey left a comment

Choose a reason for hiding this comment

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

Looks good, just a question about the installation destination stuff when packaging & a couple of small tweaks

cmake_modules/juce_helpers.cmake Outdated Show resolved Hide resolved
Comment on lines +33 to +35

# Common Files (all OS)
# note: '.' is "Setup EAR Production Suite.app/Contents/Resources" on MacOS
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is only true when run on CI? (took me a while to track it down, but I think '.' means the root of CMAKE_INSTALL_PREFIX, which is set by build.yml)
If we were to configure and install locally with EPS_PACKAGE set, does that mean we'd get everything in the CMAKE_INSTALL_PREFIX root, (and presumably the setup app would be broken?)

Is there a reason for doing it that way, or would we be better with

install(FILES
        README.pdf
        LICENSE.pdf
        DESTINATION
# note still starts with a '.' otherwise cmake will assume
# generator expression resolves to an absolute path
# also might need to escape the spaces
# also might be broken, I'm just thinking out loud :)
        .$<$<PLATFORM_ID:DARWIN>:/Setup EAR Production Suite.app/Contents/Resources>
# other install commands 

and keeping the install prefix consistent?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh wait, I just saw your comment at the top of the PR. Sorry :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it's maybe still valid as it's nice to be able to do a CI-style build locally (I don't think doing it this way negates that as the packaging file is only run if specified via a cache var). If it's hard for some reason (the post-build step suggests it probably is) then maybe keep things as they are.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Issue #275 opened to tackle local packaging at a later date

tools/setup/CMakeLists.txt Outdated Show resolved Hide resolved
@firthm01 firthm01 merged commit 809804e into main Mar 20, 2024
6 checks passed
@firthm01 firthm01 deleted the translocation-fix branch March 20, 2024 12:51
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.

2 participants