-
Notifications
You must be signed in to change notification settings - Fork 14
Rename "install/" to "pkg/" and fix macOS bundling #690
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
base: main
Are you sure you want to change the base?
Conversation
47a0627 to
da5faf9
Compare
|
The next and last step planned is to make the "unified build folder" by putting |
|
High-level summary of Host tools, UI, mac application should all be able to locate these resources when launched from under |
|
What is the rationale to put host tool in |
| third_party/freedreno/libs/ | ||
| third_party/freedreno/obj | ||
| .gradle/ | ||
| pkg/ |
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.
Put it at build/pkg?
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 want put it at pkg/ for now (same place as install/) and then move build_android/ and pkg/ under build/ in the next PR.
It's a little easier to copy files from |
scripts/deploy_mac_bundle.sh
Outdated
| done | ||
|
|
||
| # Ad-hoc signing the application bundle | ||
| codesign --force --deep --sign - ${INSTALL_DIR}/dive.app || exit 1 |
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.
Did we want to make signing optional like it was in CMake?
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.
Not sure? All I know is that on macbook you can't run your local builds right now without doing the signing, macOS will not let you run the application. I assumed this means that for mac users we currently NEED to sign. I believe it was optional in the cmakelists file because signing is not required for local builds on other platforms? @RenfengLiu probably knows more.
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.
You may want to skip signing if you will be using a different key (e.g. release process)
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.
ok makes sense, I will make it optional
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 made it optional (takes --no-sign), by default the bundle will be signed.
I also took out the parameter for the install folder name since we're now telling users to use the script to build device resources (so the name "pkg" is already used there)
5f1cbd6 to
e9e2219
Compare
|
Manual Testing (2026/01/08 4:10 EST):
|
Changes:
install/topkg/and have separate folders within for host tools, device resources, and pluginsCMAKE_GENERATED_INSTALL_DIR_PATHand predict the location of different resources relative to the location of the host tool.resource_defs.hso thatversion_defs.hdoesn't get included by many librariesBUILD.mdto reference scripts rather than repeat their contents