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 dmg support #191

Closed
wants to merge 4 commits into from
Closed

Macos dmg support #191

wants to merge 4 commits into from

Conversation

gvzdv
Copy link

@gvzdv gvzdv commented May 8, 2024

Created a .dmg file to launch Space Acres on Mac.
Created with create-dmg.
Tested on Apple Silicon; should also work on Apple Intel.
The app is not signed.
Info.plist may require adjustments.
To create an updated version, run create-dmg '/path_to/space-acres/Space-Acres.app' --overwrite

Copy link
Member

@nazar-pc nazar-pc left a comment

Choose a reason for hiding this comment

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

Thanks for taking a stab at this!

Please don't commit final binaries or those pesky .DS_Store files.

While this might work and is helpful, we'd need to automate it in CI just like it is done for Windows and Linux right now. You should be able to trigger CI in your fork to test things out.

Also test it on a fresh machine/VM, it is very suspicious that you dind't have to add any GTK libraries to dependencies, it most likely means application will fail to run on machine that doesn't have those installed, which is actually one of the main challenges of adding support for macOS.

Space-Acres.app/Contents/Info.plist Outdated Show resolved Hide resolved
<key>CFBundleName</key>
<string>Space Acres</string>
<key>CFBundleVersion</key>
<string>0.1.17</string>
Copy link
Member

Choose a reason for hiding this comment

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

Is there a way to extract it automatically from Cargo.toml? We had a single place to update version until this point, having more places is not ideal.

Copy link
Author

Choose a reason for hiding this comment

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

Not sure at this point. Will look into it.

<key>CFBundlePackageType</key>
<string>APPL</string>
<key>CFBundleSignature</key>
<string>????</string>
Copy link
Member

Choose a reason for hiding this comment

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

What is this, is it valid?

Copy link
Author

Choose a reason for hiding this comment

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

APPL is the package type for applications.

???? for CFBundleSignature was used if the creator code was unknown. I believe it's a legacy feature and can be safely removed as it's not found in the current Bundle Configuration.

Comment on lines +19 to +22
<key>CFBundleInfoDictionaryVersion</key>
<string>6.0</string>
<key>LSMinimumSystemVersion</key>
<string>10.12</string>
Copy link
Member

Choose a reason for hiding this comment

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

What do these versions correspond to?

Copy link
Author

Choose a reason for hiding this comment

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

CFBundleInfoDictionaryVersion is normally assigned by Xcode. Since this application wasn't built with Xcode, I assigned it manually. 6.0 is a common value and it should be compatible with most MacOS devices.

LSMinimumSystemVersion is the lowest MacOS version that the program can run on. I specified Sierra. Needs testing/verification.

Co-authored-by: Nazar Mokrynskyi <nazar@mokrynskyi.com>
@@ -5,7 +5,7 @@
<key>CFBundleExecutable</key>
<string>space-acres</string>
<key>CFBundleIdentifier</key>
<string>com.subspace.spaceacres</string>
<string>org.subspace.spaceacres</string>
Copy link
Member

Choose a reason for hiding this comment

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

Also I'd probably prefer space-acres if it is supported for consistency with everything else

Copy link
Author

Choose a reason for hiding this comment

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

Fixed in my branch, thank you.

@gvzdv
Copy link
Author

gvzdv commented May 9, 2024

Thank you for reviewing my PR.

I've bitten off more than I could chew with this project. It indeed doesn't work on clean machines and only launched on mine because I already had the necessary libraries installed. I'll see what I can do with it.

Noted regarding binaries and .DS_Store files.

As for the Info.plist file, I will reply to your comments to clarify some details. However, this version of the file is preliminary. When this package is properly compiled and ready for release, we'll need to take a closer look at the Info.plist file to ensure its adherence to the Apple guidelines.

@nazar-pc
Copy link
Member

I'll close this for now since we can't merge it as is. Feel free to continue discussion though and we can reopen it (or just create another PR) once more progress is done. Maybe you can also cooperate with @RomanLabGit on this, he indicated working on #7 too.

@nazar-pc nazar-pc closed this May 16, 2024
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