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

Configure correct MacOS icon (.icns file) to be used by build-mac target #127

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

thegamer87
Copy link

added MacOS icon ".icns" file (converted from a frame.gif icon in the repo used for linux) and configured app.icon.icns in project.properties to allow the replace of MacOS icon during the build of target "build-mac"

@thegamer87 thegamer87 changed the title Configure correct MacOS icon (.icns file) to be used by bundle-mac target Configure correct MacOS icon (.icns file) to be used by build-mac target Mar 20, 2023
@RadekCap
Copy link
Collaborator

Thank you for the exploration and PR.

It looks like I'm missing something. The application icon isn't present when running from Netbeans (17) or from the dist folder (Ventura 13.2.1).

Steps to reproduce:

  1. Build & run from Netbeans or
  2. Package as Zip distribution, unpack & run

The change is present in the code when building, the application icon is properly visible from Finder (not from Netbeans which doesn't support preview of icns file).

How did you manage to display a proper icon on Mac?

@thegamer87
Copy link
Author

thegamer87 commented Mar 20, 2023

Yes, I noticed this too, the build-zip target (used to package as zip distribution) on MacOS use directly .icns file from

netbeans-plat/17/harness/etc/applicationIcon.icns

while build-mac copies the file indicated in app.icon.icns property to

Contents/Resources/{app.name}.icns

and use this as icon for application, the default value of app.icon.icns is the same file in netbeans-plat, below you can see two lines from suite.xml that perform the "magic" on build-mac target (the first to define the default value of app.icon.icns, the second to perform the copy)

<property name="app.icon.icns" value="${harness.dir}/etc/applicationIcon.icns"/>
<copy file="${app.icon.icns}" tofile="${dist.dir}/${app.name}.app/Contents/Resources/${app.name}.icns"/>

and my solution is effective only for build-mac target, but I'm investigating if there is a "standard" way to customize the icon even in the case of "zip" packaging on MacOS, if I find anything I will open a new PR (to keep two scopes separate) and also post a comment here with link to the other PR

@thegamer87
Copy link
Author

I'm wrong about how application, when packaged as zip, load icon for MacOS, in the trgtd launcher (trgtd/bin/trgtd) when program is started one of parameters passed to executable is

-J-Xdock:icon=$progdir/../../$APPNAME.icns

where $progdir is the launcher path (trgtd/bin) but it is certainly not correct because $progdir/../../ go out from the program tree, I think this can be a netbeans platform bug, I've updated this PR with my proposal of solution

  1. define a custom launcher (nbproject/trgtd) that is a copy of app.sh from netbeans-plat with one replace applied
    -J-Xdock:icon=$progdir/../../$APPNAME.icns --> -J-Xdock:icon=$progdir/../harness/etc/$APPNAME.icns
    
  2. add property to project.properties
    app.launcher=nbproject/trgtd
    
  • override target build-zip in build.xml to perform two zip update operations after the zip was created (by the suite.build-zip target):
    • add the icon trgtd.icns to harness/etc
    • replace bin/trgtd with customized launcher from nbproject

@ursjoss
Copy link
Collaborator

ursjoss commented Mar 22, 2023

@thegamer87 @RadekCap I'd like to avoid maintaining a copy of the Netbeans provided app.sh. I'd like to suggest following a different path:

There already is a dedicated ant build file for creating the application for mac: build-osx.xml. It contains a number of very interesting ant targets:

I tried running ant -f build-osx.xml pkg-osx-exclude-jre, as that currently corresponds to what we are trying to do with Netbeans stock installer creation.

The target relies on some files that need to be present in the following path nbproject/packaging/macosx/:

  • trgtd.conf (mac specific variant I guess)
  • trgtd.icns

Jeremy configured that target to prepare dist/ThinkingRock/ThinkingRock.app, ensuring we have all the relevant stuff in (including the trgtd.icns) and even cares to remove non-Mac related stuff (windows/linux native infrastructure we don't need for Mac). Eventually it fails for me, as I don't have /usr/bin/pkgbuild available on Linux. I understand it is available (or easily installable) on Mac and is used to create the App binary that can then be installed on Mac via Double-Click. There may be additional obstacles after this failure, but we will only know if we try it on Mac. I'ts probably not worthwhile creating a workaround to let us create mac installers on linux for now.

The path nbproject/packaging/ is ignored (in .gitignore). We may choose to not ignore that path and actually commit those files - or not if there are valid reasons.

Now for the trgtd.icns: @thegamer87 The one you have created seems to not be fully compliant with the specs. I noticed it using inkscape to open the file. It works well with the one provided by Netbeans but fails with your version. I found a reference that only certain png sizes should be included in the icns file: 16x16, 32x32, 128x128, 256x256, 512x512 and 1024x1024 (see here). I managed to create such an trgtd.icns file that can be opened using inkscape. However, I suggest we re-use the trgtd.icns file that was bundled with the installer of thinking rock 3.7.0.

The same for the trgtd.conf file: Let's use one that comes with 3.7.0 and let's see if and how it differs from the one in nbproject.

Would you be able to follow this path and see how far we get?

@thegamer87
Copy link
Author

thegamer87 commented Apr 3, 2023

Hi,
sorry for the delay but my daughter (fortunately) doesn't leave me much time for passions XD

I've tried the build through

ant -f build-osx.xml pkg-osx-exclude-jre

after several "trial and error" I got to the end of the build (attached the output of ls command that shown nbproject/packaging/macosx folder with comments indicating where the files came from ... if you have different suggestions regarding these they are obviously welcome), so I've installed the package created by the build and the good news is that the application has the correct icon.
The only question left open is that the installed application does not start unless you open it from the terminal via open APPLICATION command because only in this way the jdkhome environment variable is correctly read while double clicking through finder doesn't read it (and this seems to be a mac os x design thing), do you have any idea how we could overcome this problem?
tr

@ursjoss
Copy link
Collaborator

ursjoss commented Apr 3, 2023

Thanks for digging into this @thegamer87. Don't worry about delays, especially not if it's about spending time with your daughter.

I think it would be helpful if you could push your current state, so others with Mac can have a look. If you think, this path is the right one, you could revert the first two commits in this PR (we can still get it back if needed later) and add the current state as one or more new commits. If you want to follow both paths in parallel, you could create a new PR from a feature branch in your repo that does not include the first two commits.

I don't work on Macs, so I don't have lot's to contribute here, but a few questions:

Thanks a lot for your contributions.

@thegamer87
Copy link
Author

Yes I can for sure revert my older commit, for the current state the only files added are into nbproject/packaging/macosx so can I remove this folder from .gitignore and commit it?
Regarding the question directed to me : Yes I can launch the installer from finder.

@thegamer87
Copy link
Author

I've reverted my older commits and removed from gitignore and committed the folder "nbproject/packaging/macosx".

@ursjoss
Copy link
Collaborator

ursjoss commented Apr 22, 2023

@thegamer87 Thanks for applying those changes. They are a good starting point for further evaluation. I would assume that with the current files, it does not work yet.

For a starter, the config files in nbproject/packaging/macosx/etc-exclude-jre/ would need to be adjusted. The default_mac_userdir should be changed to point to tr-4.0 instead of tr-3.7. Then we need to tweak the default_options. We need to include at least parts of what you can find in nbproject/trgtd.conf , as some of those options were necessary for running ThinkingRock with java 17. Possibly some of the current options may be Mac specific. If so, they should probably be kept.

Somebody with a mac should simply try out if

ant -f build-osx.xml pkg-osx-exclude-jre

runs successfully and if the resulting installer is functional. Until that works, those settings need tweaking. I am not a Mac user, so I hope that others will invest time here...

@thegamer87
Copy link
Author

Yes, I confirm (like I wrote two mine comments above) that with those files the build succeed and the installer works on my Mac. Maybe another Mac user can make a try (@degrown for example)

@ursjoss
Copy link
Collaborator

ursjoss commented Apr 22, 2023

Yes, I confirm (like I wrote two mine comments above) that with those files the build succeed and the installer works on my Mac. Maybe another Mac user can make a try (@degrown for example)

Sorry, I missed that statement. But that's good news.

I am somewhat surprised that the default_options without all the --add-opens statements (see here) work. Are you sure those files are actually used in the build?

Also, I'm not sure where default_mac_userdir is actually used. I would have started with the following entries in the trgtd.conf files:

default_userdir="${DEFAULT_USERDIR_ROOT}/tr-4.0"
default_cachedir="${DEFAULT_CACHEDIR_ROOT}/tr-4.0"
app.icns=branding/core/core.jar/org/netbeans/core/startup/trgtd.icns

I add it here just as a reference.

@ursjoss
Copy link
Collaborator

ursjoss commented Apr 26, 2023

Thanks verroç, for your update. I am suspecting that the path to the license file is not matching as it is, but that should probably be verified while running the build...

@RadekCap and/or @degrown, would you be able to jump in testing this PR?

@thegamer87
Copy link
Author

Yes, I confirm (like I wrote two mine comments above) that with those files the build succeed and the installer works on my Mac. Maybe another Mac user can make a try (@degrown for example)

Sorry, I missed that statement. But that's good news.

I am somewhat surprised that the default_options without all the --add-opens statements (see here) work. Are you sure those files are actually used in the build?

Also, I'm not sure where default_mac_userdir is actually used. I would have started with the following entries in the trgtd.conf files:

default_userdir="${DEFAULT_USERDIR_ROOT}/tr-4.0"
default_cachedir="${DEFAULT_CACHEDIR_ROOT}/tr-4.0"
app.icns=branding/core/core.jar/org/netbeans/core/startup/trgtd.icns

I add it here just as a reference.

in fact nbproject/trgtd.conf is not used but .conf files used during the build are into nbproject/packaging/macosx/etc-exclude-jre

➜  tr-pc git:(main) ls -al nbproject/packaging/macosx/etc-exclude-jre
total 40
drwxr-xr-x  7 verrocchiom  staff  224 Apr 26 16:18 .
drwxr-xr-x  5 verrocchiom  staff  160 Apr 11 16:08 ..
-rw-r--r--  1 verrocchiom  staff  328 Apr 26 16:18 trgtd-deDE.conf
-rw-r--r--  1 verrocchiom  staff  328 Apr 26 16:18 trgtd-enUS.conf
-rw-r--r--  1 verrocchiom  staff  328 Apr 26 16:18 trgtd-esES.conf
-rw-r--r--  1 verrocchiom  staff  328 Apr 26 16:18 trgtd-frFR.conf
-rw-r--r--  1 verrocchiom  staff  313 Apr 26 16:18 trgtd.conf

@ursjoss
Copy link
Collaborator

ursjoss commented Apr 26, 2023

in fact nbproject/trgtd.conf is not used but .conf files used during the build are into nbproject/packaging/macosx/etc-exclude-jre

That's certainly true if you trigger ant with the config file build-osx.xml, the main purpose of this PR. I did not check in detail, but most likely nbproject/trgtd.conf is used for other kinds of builds. If not, we could remove it.

But my main point was rather: I did not find the place where the property default_mac_userdir is actually used. I am rather confident that default_userdir is used by the build. It may be worthwhile experimenting by running ant with one or the other configuration property and examining if the resulting artifact behaves differently.

@degrown
Copy link
Contributor

degrown commented Apr 27, 2023

Thanks verroç, for your update. I am suspecting that the path to the license file is not matching as it is, but that should probably be verified while running the build...

@RadekCap and/or @degrown, would you be able to jump in testing this PR?

Here we are, I'm again recovering past discussions...
If someone could generate zip/app Mac artifacts for me, I could test it today
In the meanwhile, thank you @thegamer87 and @ursjoss for your hard and subtle work about that!

@ursjoss
Copy link
Collaborator

ursjoss commented Apr 27, 2023

Thanks verroç, for your update. I am suspecting that the path to the license file is not matching as it is, but that should probably be verified while running the build...
@RadekCap and/or @degrown, would you be able to jump in testing this PR?

Here we are, I'm again recovering past discussions... If someone could generate zip/app Mac artifacts for me, I could test it today In the meanwhile, thank you @thegamer87 and @ursjoss for your hard and subtle work about that!

I cannot provide the installer, unfortunately, as the current ticket requires a Mac to build it.

@degrown
Copy link
Contributor

degrown commented Apr 27, 2023

Thanks verroç, for your update. I am suspecting that the path to the license file is not matching as it is, but that should probably be verified while running the build...
@RadekCap and/or @degrown, would you be able to jump in testing this PR?

Here we are, I'm again recovering past discussions... If someone could generate zip/app Mac artifacts for me, I could test it today In the meanwhile, thank you @thegamer87 and @ursjoss for your hard and subtle work about that!

I cannot provide the installer, unfortunately, as the current ticket requires a Mac to build it.

Thank you and don't worry about that...I'll ask to @thegamer87, we'll find a way to do it.

@RadekCap
Copy link
Collaborator

@degrown I'll prepare ZIP distribution for testing.

@thegamer87
Copy link
Author

I've an update: I've tried to uninstall openjdk (installed via homebrew) and installed JDK (20) from https://www.oracle.com/java/technologies/downloads/#jdk20-mac (pkg installer)

with this jdk tr application installed through installer generated by
ant -f build-osx.xml pkg-osx-exclude-jre

can be launched directly by double click on application (with openjdk the application could only be launched from a terminal with jdkhome env var set).

I'll share my tr installer with @degrown and we perform some tries on his mac.

@ursjoss
Copy link
Collaborator

ursjoss commented Mar 17, 2024

@thegamer87 @degrown What will be the next steps to be able to advance this issue? What were your findings when testing together?

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.

4 participants