Community
Participate
Working Groups
Build: 4.3.0.I20130227-2000 The eclipse-SDK-I20130227-2000-macosx-cocoa.tar.gz download. I noticed that the Eclipse.app does not show the proper Icon in Finder. Looking at the Eclipse.app/Contents/Info.plist file I see the following: <key>CFBundleIconFile</key> <string></string> Previously this included the Eclipse.icns string.
I think the Info.plist content is generated based of the following file in the rt.equinox.framework repo: http://git.eclipse.org/c/equinox/rt.equinox.framework.git/tree/bundles/org.eclipse.equinox.executable/bin/cocoa/macosx/x86/Eclipse.app/Contents/Info.plist As you can see from that source the CFBundleIconFile is specified there correctly, so if this file is what is used as a template for the one included in the build then something removed the original icon specification.
*** Bug 402056 has been marked as a duplicate of this bug. ***
There is code in the p2 BrandingIron to replace the CFBundleIconFile value, though that same code is used for replacing other elements in the Info.plist.
*** Bug 405060 has been marked as a duplicate of this bug. ***
Doug Schaefer, I can't remember where, but I think it was you that I saw comment on some mailing list or forum that you (CDT) got getter results with Tycho 0.18-snapshot. (We in Platform are using 0.17 release). So ... 1) sound familiar? Was it you? and if so, 2) does it sound related to bugs such as this one?
Created attachment 230425 [details] grep of output logs for Eclipse.icns
Created attachment 230426 [details] grep of output logs for Info.plist.txt
I know our logs are huge, especially the 400 MB one from Tycho/Maven running in debug mode ... so, thought I'd grep them for 'Eclipse.icns' and again for 'Info.plist'. But, these lines don't mean much to me. Help anyone else spot the problem area? Tom, I notice in build.properties, you have only bin.includes for the resources .... I know in some cases, Tycho/Maven expects the resources in a "sources" directory, achieved with src.includes So, it might be a matter of "timing" ... if Tycho/Maven (recreates) the PList.info., based on what it finds in "sources", it might conclude there isn't an icon and leave it blank. Pretty wild guess, but ... only thing I can think of.
Thanh, have you heard of any issues with "Mac builds" that might be related to this? I briefly searched Tycho's bug list, but didn't see anything.
Last "data" :) ... On the build machine itself, I did a "find" for Eclipse.icns and found following: [15:17:29] david_williams@build:/opt/public/eclipse/builds/4I/gitCache/eclipse.platform.releng.aggregator/rt.equinox.framework/features/org.eclipse.equinox.executable.feature/target $ find ./ -name Eclipse.icns ./tmp/bin/cocoa/macosx/x86/Eclipse.app/Contents/Resources/Eclipse.icns ./tmp/bin/cocoa/macosx/ppc/Eclipse.app/Contents/Resources/Eclipse.icns ./tmp/bin/cocoa/macosx/x86_64/Eclipse.app/Contents/Resources/Eclipse.icns ./tmp/bin/carbon/macosx/x86/Eclipse.app/Contents/Resources/Eclipse.icns ./tmp/bin/carbon/macosx/ppc/Eclipse.app/Contents/Resources/Eclipse.icns Tom, so you have a custom "ant run" that's supposed to copy these to some special place? I would not know where that would be "to", but this list might at least show you where they need to be copied "from" :) You can browse this build site with your web browser, starting with http://build.eclipse.org/eclipse/builds/4I/gitCache/eclipse.platform.releng.aggregator/rt.equinox.framework/features/org.eclipse.equinox.executable.feature/target/
CC'ing Paul since he setup the build for org.eclipse.equinox.executable.feature. I'm not really sure why this feature would cause issues for the app folder that ultimately gets created for the eclipse project though.
(In reply to comment #9) > Thanh, have you heard of any issues with "Mac builds" that might be related > to this? > Unfortunately not, I haven't heard of anything that might cause this.
According to CBI List, this may be part of the "p2.publisher" which Tycho Builder uses ... so considering making a fix for https://bugs.eclipse.org/bugs/show_bug.cgi?id=406942 and put in Tycho 0.18-Snapshot (if they haven't yet) ... and, then make "final version" of Tycho builder in a few weeks. If I'm reading the CBI list correctly. http://dev.eclipse.org/mhonarc/lists/cbi-dev/msg00998.html
Ah, I see 406942 already listed as a "depends on" bug.
> Ah, I see bug 406942 already listed as a "depends on" bug. Sorry, I should have posted this explicitly here (or better: Bugzilla should have added a comment here on external additions to "Depends on:" and "Blocks:"). I don't think bug 406942 is the ultimate culprit for this bug, but it could at least help avoid the missing icon. I think the real bug is that in the "branding" step (where other parts of the product are branded), the Mac icns is missing. However, I have no clue where this "branding" step is triggered, nor how to influence it.
*** Bug 407222 has been marked as a duplicate of this bug. ***
Anyone care to experiment? I noticed in our "configuration" (fake) feature, there was a bunch of commented out lines related to "macos". I assumed there was a reason for it. But ... maybe it was just some "unfinished" work we inherited from the prototype we were provided. If it works, it will make a difference in the p2 metadata for the "product". http://git.eclipse.org/c/platform/eclipse.platform.releng.aggregator.git/commit/?id=81fa4b8973b3ed3582aefbc88329989b804de515 There appears to be a slightly different form of this "root" property, that copies some stuff into a "resources" directory. It seems that's only used for "linux" distributions currently, but it contains stuff like "about.html" files. Perhaps the icon also needs to go in that "resources" directory, so Tycho can "find" it when it goes to create its own plist?
(In reply to comment #17) > Anyone care to experiment? I noticed in our "configuration" (fake) feature, > there was a bunch of commented out lines related to "macos". I assumed there > was a reason for it. But ... maybe it was just some "unfinished" work we > inherited from the prototype we were provided. > > If it works, it will make a difference in the p2 metadata for the "product". > > http://git.eclipse.org/c/platform/eclipse.platform.releng.aggregator.git/ > commit/?id=81fa4b8973b3ed3582aefbc88329989b804de515 > I missed a few periods in first commit. http://git.eclipse.org/c/platform/eclipse.platform.releng.aggregator.git/commit/?id=f2621b83ce1555d4b88f4840092c634a266942a6 They should both be reverted, if we revert them. I'd already started a test build, but we could "look" at x86_64 version (and if correct there, but incorrect for x86, we'd know we'd found the answer! (ha, unlikely ... this is sadly "trial and error").
(In reply to comment #17) > Anyone care to experiment? I noticed in our "configuration" (fake) feature, > there was a bunch of commented out lines related to "macos". I assumed there > was a reason for it. But ... maybe it was just some "unfinished" work we > inherited from the prototype we were provided. > There was a reason for it ... I guess. My test build fails process that file with an error message similar to p2-metadata-default failed: root.macosx.cocoa.x86.folder.Eclipse.app/Contents/MacOS has too many segments Not sure, if that's a "p2 limitation" ... or ... a Tycho bug? But, I'll revert the changes, and perhaps experiment more later.
(In reply to comment #19) > (In reply to comment #17) > There was a reason for it ... I guess. My test build fails process that file > with an error message similar to > > p2-metadata-default failed: > root.macosx.cocoa.x86.folder.Eclipse.app/Contents/MacOS has too many segments > > Not sure, if that's a "p2 limitation" ... or ... a Tycho bug? > It is a Tycho "limitation" ... my experiment has been tried before, right Thanh? :) (but, admittedly a year ago!) http://dev.eclipse.org/mhonarc/lists/cbi-dev/msg00395.html I can try to implement "the workaround" mentioned in that message, but their example is so much simpler than our build, I'm not sure I can get it to work "end to end" ... at least, in a few days or weeks ... but, will explore. I have opened a separate entry, bug 407703, where I will track work specific to "re-doing" that "special feature" ... and report back here if/when I have a solution.
I thought I was making progress, since after several changes for bug 407703 (which included reverting bug 401037) the Icon was showing up for the Platform (only) binary distribution ... but, then discovered that did not work or show up for Eclipse SDK. THEN, noticed, even for the Platform binary distribution, even though Icon is there in Info.plist ... but, it is identified as "Eclipse 3.7"! Is that a hint? What I tried to do, in "configuration feature", was to copy the Eclipse.app resources from rt.equinox.binaries/org.eclipse.equinox.executable/bin/cocoa/macosx/x86_64 And now that I look in Git, it does indeed say "3.7"! So ... if that's is something that is "corrected" during the build, then I'm copying too early! It is set up to do the copy during the "validate" phase ... that's the way I found existing ones ... maybe is should be done during "build" phase? Does anyone know for sure these binaries are "updated" during the build? No time for another test build tonight, so will leave things as they are for tonight's I-build ... but correcting the <phase> might make platform "current" and might make SDK "work"?
(In reply to comment #21) > > What I tried to do, in "configuration feature", was to copy the Eclipse.app > resources from > > rt.equinox.binaries/org.eclipse.equinox.executable/bin/cocoa/macosx/x86_64 > Oh, I see from Tom's comment 1 this should be rt.equinox.framework/bundles/org.eclipse.equinox.executable/bin/cocoa/macosx/x86_64 Well ... I can try that for tonight's I-build.
(In reply to comment #22) > (In reply to comment #21) > > rt.equinox.framework/bundles/org.eclipse.equinox.executable/bin/cocoa/ > macosx/x86_64 > > Well ... I can try that for tonight's I-build. The good news, is I do see the "4.3" numbers, now, with corrected file path, the bad news is getting same result on icon ... it is "filled in" for binary platform, it is not filled info full SDK. (Note, while we don't "publish" it, we do build the platform SDK and on build machine, I do see that it does also have the icon "filled in" ... so, it's not just a matter of "source vs. no source".). Not sure what the "full SDK" is doing (or needs) differently.
*** Bug 407863 has been marked as a duplicate of this bug. ***
In case it matters. The icon file itself is still there as it is in eclipse 4.2.2: $ ls -l eclipse*/Eclipse.app/Contents/Resources/; diff -s eclipse*/Eclipse.app/Contents/Resources/*icns eclipse/Eclipse.app/Contents/Resources/: total 560 -rw-r--r--@ 1 claudio staff 286605 May 2 16:21 Eclipse.icns eclipse4.2.2/Eclipse.app/Contents/Resources/: total 560 -rw-r--r-- 1 claudio staff 286605 Feb 4 12:38 Eclipse.icns Files eclipse/Eclipse.app/Contents/Resources/Eclipse.icns and eclipse4.2.2/Eclipse.app/Contents/Resources/Eclipse.icns are identical it is just the Info.plist that has changed. But there is something else that is strange. I modified the Info.plist and added the Eclipse.icns, yet the icon did not change for the better. I have not yet tried to reboot, but removing all .DS_Store files and relaunching the Finder did not make any change either. I also changed the CFBundleName from eclipse back to Eclipse as it was in 4.2.2 and then the CFBundleIdentifier from org.eclipse.eclipse to org.eclipse.eclipse as it was in eclipse 4.2.2 but none of these changes had any effect. As soon as I can reboot my Mac I will report if something changes or not.
I don't think it's the .DS_Store that caches app icons, but some other cache that is not refreshed when Info.plist is modified. The icon appears after you move the Eclipse.app somewhere else or make a copy. Renaming alone is not enough.
Thanks Markus for the hint. By copying the eclipse directory with different versions of the Info.plist to different new directories I could verify that the empty CFBundleIconFile entry in the Info.plist is the sole culprit. Once this is replaced with <string>Eclipse.icns</string> the proper icon is shown.
(In reply to comment #27) > Thanks Markus for the hint. By copying the eclipse directory with different > versions of the Info.plist to different new directories I could verify that > the empty CFBundleIconFile entry in the Info.plist is the sole culprit. Once > this is replaced with <string>Eclipse.icns</string> the proper icon is shown. This sounds promising: http://stackoverflow.com/questions/5452111/restart-refresh-dock-killall-dock-cache I haven't tried this personally, but it sounds plausible :)
So, you know how the icon was in Info.plist for the platform, but not in the SDK ... just to remind you of the "current state" of things, from my point of view. I noticed in the "sdk .product definition" we had (only) following launcher definition, for both "platform" and "SDK". <launcher name="eclipse"> <solaris/> <win useIco="false"> <bmp/> </win> </launcher> We always have had it that way (even for PDE based builds) ... but, on a whim, (after seeing some failures, actually, for "starter kit" product) I thought I'd experiment (on my local test machine) and try following (so far, in SDK product only). <launcher name="eclipse"> <macosx icon="Eclipse.app/Contents/Resources/Eclipse.icns"/> <solaris/> <win useIco="false"> <bmp/> </win> </launcher> And, now ... at least for one test build ... the icon is "filled in" in the Info.plist for the SDK distribution. Does this "make sense" to anyone? Perhaps working around some other bug? Random luck? Other experimental changes I've been trying? Or, should it always be defined in the launcher? (I know if you use the "product editor" it fills in a complete, absolute path of where to find the icon ... but, pretty sure that would not work for our Tycho based build).
> And, now ... at least for one test build ... the icon is "filled in" in the > Info.plist for the SDK distribution. > > Does this "make sense" to anyone? Perhaps working around some other bug? > Random luck? Other experimental changes I've been trying? Or, should it > always be defined in the launcher? > There was another bug that Paul and I found where we didn't have things specified in the .product file and it worked with PDE/Build but not Tycho. For backwards compatibility (before .product files existed), PDE/Build may have been doing some 'magic' when things were not specified. Of course Tycho doesn't do that magic. I now have the CBI Build running on my machine (a Mac). I can give this whole build a spin tomorrow and see if the icons show up (unless there is build scheduled before then).
I've tried adding to all our ".product" definitions and am trying a test build now. http://git.eclipse.org/c/platform/eclipse.platform.releng.aggregator.git/commit/?id=29a5ec1a6fca61ba3d8e94809c0e0463fe7359ec It still "feels like" this is working around some bug, but ... if it works ... but will revert if it does not work. Paul, care to review/comment?
As Ian mentioned, PDE seems to have some slightly different defaults for non-specified situations. If this fixes the problem it looks like a reasonable change (I don't know if it fixes the problem :-) PW
Test build completed, at http://build.eclipse.org/eclipse/builds/4I/siteDir/eclipse/downloads/drops4/I20130514-0919/ But it did not fix the problem. Same as before "platform packages" have it, "SDK packages" do not. I will revert the change.
I've managed to get a local CBI Build of Eclipse that includes icons on Mac.. yeah! :-). Here is what I did, and here is my theory on what's going on. What I did: I did the same thing as David, I added the <macosx icon="Eclipse.icns"/> to the product file. Brian de Alwis gave me a tip that Tycho's image lookup is a little different than PDE/Build, so I also added the Eclipse.icns file as siblings to the product files. We can hopefully do a bit better than this. What's going on: As David mentioned, this already worked with the platform build, but not with the SDK. Here is my theory on why this is. We currently have two sets of 'root files' (those files that get placed at the root of an Eclipse install). One set comes as part of the executable feature and the other comes as part of the 'root file artifact' (this includes things like about files, etc...). Ideally the contents of these two binary payloads (artifacts) should be mutually exclusive, but unfortunately they're not. In particular, the 'root files' payload contains some of the files from the executable's payload. Stepping back a bit. The Executable Feature starts with some reasonable defaults. That is, the Info.plist on Mac contains the Eclipse Icon. During the build, when Tycho (or likely the p2 publisher runs), it sets the image file to blank "" on MacOS if nothing is specified. On Windows and Linux there is no equivalent to the Info.plist, or more to my point, no easy way to set the icon to blank. Now it appears that the 'root files payload' contains the 'default executable files' and the 'executable payload' contains the branded one (and in our case, branded means blank). Now putting this all together. We now have two different set of root files -- one with branded Executables (remember, on MacOS, branded means no icons because none were specified), and non-branded ones (which actually contain the icons, as reasonable defaults). At install time it's anyones guess which ones will get written to disk (on the platform it appears the root files is written second, thus 'winning' whereas on the SDK, the executable payload is written second). What do we do: 1. We start by properly branding the macOS installs -- that is, including the images in the .product files. 2. We fix the publisher so that blank means Keep the Defaults. 3. We sort out why we have two overlapping sets of Root Files.
(In reply to comment #34) > > What do we do: > > 1. We start by properly branding the macOS installs -- that is, including > the images in the .product files. So you mean literally Eclipse.icns not the full Eclipse.app/Contents/Resources/Eclipse.icns that I tried before? I can add add for tonight's build, since you are confident that is one of the steps needed. > 2. We fix the publisher so that blank means Keep the Defaults. This means p2 publisher? Opened a bug yet? :) I assume that's no help with current release. > 3. We sort out why we have two overlapping sets of Root Files. This one I don't quite understand. We do name several features in our products, and several of them specify 'rootfiles', but not all of them. But, I don't think that's what you mean, right?
(In reply to comment #35) > (In reply to comment #34) > > > > > What do we do: > > > > 1. We start by properly branding the macOS installs -- that is, including > > the images in the .product files. > > So you mean literally > Eclipse.icns > not the full > Eclipse.app/Contents/Resources/Eclipse.icns > that I tried before? Yes, but I also literally copied the Eclipse.icns file to the directories that contained the .product files. We have about 6 .product files so the icns file was copied 6 times :-(. The publisher / Tycho takes care of putting it in the right place (back under <AppName>.app/Contents/Resources/. > > I can add add for tonight's build, since you are confident that is one of > the steps needed. > > > 2. We fix the publisher so that blank means Keep the Defaults. > > This means p2 publisher? Opened a bug yet? :) I assume that's no help with > current release. You are correct, it won't be any help with the current release. I filed bug#408166. > > > 3. We sort out why we have two overlapping sets of Root Files. > > This one I don't quite understand. We do name several features in our > products, and several of them specify 'rootfiles', but not all of them. But, > I don't think that's what you mean, right? If you look in rcp.config (this appears to be a feature we build) it includes root files. One of the root files (on Mac) is Eclipse.App/Resources/<blah>. This means that at Install time, all these files will get put on disk. If we change the Eclipse name to FooBar next week, we will install install Eclipse.App/<blah>. These files get written in addition to the ones from the Executable feature. If I get a chance later today, I will try removing the Eclipse.app stuff from rcp.config, and see what happens.
(In reply to comment #36) > (In reply to comment #35) > > (In reply to comment #34) > > Yes, but I also literally copied the Eclipse.icns file to the directories > that contained the .product files. We have about 6 .product files so the > icns file was copied 6 times :-(. The publisher / Tycho takes care of > putting it in the right place (back under <AppName>.app/Contents/Resources/. > From what you've said, it probably won't help, but for tonight's build, I have put the "spec" back in the .product files, just as icon name, no paths. http://git.eclipse.org/c/platform/eclipse.platform.releng.aggregator.git/commit/?id=74408376435289187c764eb09f066bd13ac9c4a6 But, I did not "make copies" of the actual Eclipse.icns files. I'd prefer not too unless it's confirmed that is absolutely required. It would seem to me, "copying one icon into multiple places", should be the same thing as "having multiple copies in the source repo". No? And, if we copy them anywhere, it should probably be with the main, "defining feature" for that product. The "config feature" is something causing us lots of grief in Tycho and we don't want it to be included, or "show up" (it used to not "show up"). See bug 401037 and bug 407703. But ... to "get rid of it" right now, would likely be a bit risky. I might remind everyone, I saw it work once too ... in one of my test builds ... but, then did not work on build.eclipse.org ... so ... does seem like there is something very complicated going on.
(In reply to comment #37) > From what you've said, it probably won't help, but for tonight's build, I > have put the "spec" back in the .product files, just as icon name, no paths. > > http://git.eclipse.org/c/platform/eclipse.platform.releng.aggregator.git/ > commit/?id=74408376435289187c764eb09f066bd13ac9c4a6 > > But, I did not "make copies" of the actual Eclipse.icns files. I'd prefer > not too unless it's confirmed that is absolutely required. It would seem to > me, "copying one icon into multiple places", should be the same thing as > "having multiple copies in the source repo". No? > From what I can tell, the icon files need to be relative to the .product file. So we should (in theory) be able to put them in one location, and do something like ../../../foo/bar/Eclipse.icns. I put them next to the .product to limit the number of things going on in my test.
All fixed, eh? (In the interest of full disclosure, I also removed a previous bit of work-around code that _might_ have been causing unknown side effects ... see bug 407703 comment 3 ... so as far as I know, we could take out the explicit naming now and it'd would still work! (Not that I'm suggesting that ... just editorializing :).
(In reply to comment #36) > (In reply to comment #35) > > (In reply to comment #34) > > > 2. We fix the publisher so that blank means Keep the Defaults. > > > > This means p2 publisher? Opened a bug yet? :) I assume that's no help with > > current release. > > You are correct, it won't be any help with the current release. I filed > bug#408166. FYI: There may will be another Tycho release in the Kepler timeframe, and there have been requests to update Tycho's p2 version: see bug 407972. So if you want to fix the p2 bug now, you probably also want to join that bug.
(In reply to comment #39) > All fixed, eh? > > (In the interest of full disclosure, I also removed a previous bit of > work-around code that _might_ have been causing unknown side effects ... see > bug 407703 comment 3 ... so as far as I know, we could take out the explicit > naming now and it'd would still work! (Not that I'm suggesting that ... just > editorializing :). FWIW, there is a number of "FileNotFoundExceptions" in the log, that look like that below. So, while it does seem to currently "work" ... doubt we have fixed in exactly the right way ... and by "work", I mean Info.plist file contains Eclipse.icns. = = = = = java.io.FileNotFoundException: /data/shared/eclipse/builds/4I/gitCache/eclipse.platform.releng.aggregator/eclipse.platform.releng.tychoeclipsebuilder/platform.sdk/target/products/org.eclipse.platform.sdk/Eclipse.icns (No such file or directory) at java.io.FileInputStream.open(Native Method) at java.io.FileInputStream.<init>(FileInputStream.java:138) at org.eclipse.pde.internal.publishing.Utils.copy(Utils.java:27) at org.eclipse.equinox.internal.p2.publisher.eclipse.BrandingIron.brandMac(BrandingIron.java:229) at org.eclipse.equinox.internal.p2.publisher.eclipse.BrandingIron.brand(BrandingIron.java:121) at org.eclipse.equinox.p2.publisher.eclipse.EquinoxExecutableAction.fullBrandExecutables(EquinoxExecutableAction.java:275) at org.eclipse.equinox.p2.publisher.eclipse.EquinoxExecutableAction.brandExecutables(EquinoxExecutableAction.java:248) at org.eclipse.equinox.p2.publisher.eclipse.EquinoxExecutableAction.perform(EquinoxExecutableAction.java:67) at org.eclipse.equinox.p2.publisher.eclipse.ApplicationLauncherAction.perform(ApplicationLauncherAction.java:66) at org.eclipse.equinox.p2.publisher.eclipse.ProductAction.perform(ProductAction.java:100) at org.eclipse.equinox.p2.publisher.Publisher$ArtifactProcess.run(Publisher.java:207) at org.eclipse.equinox.p2.repository.artifact.spi.AbstractArtifactRepository.executeBatch(AbstractArtifactRepository.java:187) at org.eclipse.tycho.repository.module.ModuleArtifactRepositoryDelegate.executeBatch(ModuleArtifactRepositoryDelegate.java:93) at org.eclipse.equinox.p2.publisher.Publisher.publish(Publisher.java:231)
(In reply to comment #41) Yeah, I think after the "fix" from comment 37, BrandingIron#brandMac(..) fails early enough so that the whole branding is cancelled and we don't run into bug 406942 any more. After the "for" loop from line 209, the "File icon" contains a non-existent file, which makes the Utils.copy operation fail. EquinoxExecutableAction #fullBrandExecutables(..) catches the exception and runs e.printStackTrace(). The result is that a few other properties in Info.plist are also not updated. E.g. CFBundleIdentifier is now "org.eclipse.eclipse" and not "org.eclipse.sdk.ide" any more.
(In reply to comment #42) > (In reply to comment #41) > Yeah, I think after the "fix" from comment 37, BrandingIron#brandMac(..) > fails early enough so that the whole branding is cancelled and we don't run > into bug 406942 any more. > > After the "for" loop from line 209, the "File icon" contains a non-existent > file, which makes the Utils.copy operation fail. EquinoxExecutableAction > #fullBrandExecutables(..) catches the exception and runs e.printStackTrace(). > > The result is that a few other properties in Info.plist are also not > updated. E.g. CFBundleIdentifier is now "org.eclipse.eclipse" and not > "org.eclipse.sdk.ide" any more. Yes, I agree with this. I just checked and because the icons are not relative to the .product file, whole branding is aborted and we get the defaults. And like you Markus, I suspect the root of the problem is bug 406942. I don't mind applying your patch there, but do we really want tycho to consume a new p2 at this point, and for us to consume a new Tycho? Are we happy with that risk at this stage?
(In reply to comment #43) > (In reply to comment #42) > > (In reply to comment #41) > > And like you Markus, I suspect the root of the problem is bug 406942. I > don't mind applying your patch there, but do we really want tycho to consume > a new p2 at this point, and for us to consume a new Tycho? Are we happy with > that risk at this stage? I would. Under the circumstances. We are already consuming a lot of "last minute" fixes in 0.18.0-SNAPSHOT ... so, already a little risky. You would know better than I if the p2 changes themselves are risky ... but, just letting you know we are depending on other "last minute" fixes ... unfortunately.
After bug 406942 is in and Tycho uses the new p2, comment 37 should be reverted (to stop the early failure of the branding step).
(In reply to comment #45) > After bug 406942 is in and Tycho uses the new p2, comment 37 should be > reverted (to stop the early failure of the branding step). But in the meantime we should update the icon location so it actually finds a proper icon.
(In reply to comment #46) > (In reply to comment #45) > > After bug 406942 is in and Tycho uses the new p2, comment 37 should be > > reverted (to stop the early failure of the branding step). > > But in the meantime we should update the icon location so it actually finds > a proper icon. Well, there is one in the rootfiles, at Eclipse.app/Contents/Resources/Eclipse.icns Which is what I tried before. So ... you are saying I should change it "back" to Eclipse.app/Contents/Resources/Eclipse.icns Guess I can try that in a test build (in my test branch) and see if then there are no more "FileNotFoundExceptions".
(In reply to comment #47) > (In reply to comment #46) > > (In reply to comment #45) > > > After bug 406942 is in and Tycho uses the new p2, comment 37 should be > > > reverted (to stop the early failure of the branding step). > > > > But in the meantime we should update the icon location so it actually finds > > a proper icon. > > Well, there is one in the rootfiles, at > Eclipse.app/Contents/Resources/Eclipse.icns > > Which is what I tried before. So ... you are saying I should change it > "back" to > Eclipse.app/Contents/Resources/Eclipse.icns > > Guess I can try that in a test build (in my test branch) and see if then > there are no more "FileNotFoundExceptions". So, for the record, adding (only) <macosx icon="Eclipse.app/Contents/Resources/Eclipse.icns"/> Still resulted in "file not found" Exception. I guess the product creation does not look in root files ... or, is "created" before root files are in place? Oddly, just putting a copy of Eclipse.icns directly in *.product folder seemed to suffice ... that is, leaving <macosx icon="Eclipse.app/Contents/Resources/Eclipse.icns"/> as it such. So ... still not sure of the correct action to take, here. Assuming we copied Eclipse.icns to each *.product folder, that is the correct value of <macosx icon="..."/> ? I guess the question is, does <macosx icon="..." /> specify one place to look for icon? Or where icon is supposed to be copied to?
David: the .product icon attributes specify where the icon is found, not where it should be placed. Unfortunately PDE/Build and Tycho differ in how they resolve the icons: Tycho seems to resolve the icons from the target/products/<product.id> directory. In my Tycho-based apps, I use an maven-antrun-plugin job to copy the icon into place. With Kizby, my product file included: <splash location="ca.mt.kizby.app" /> <launcher name="Kizby"> <linux icon="ca.mt.kizby.app/icons/Kizby.xpm"/> <macosx icon="ca.mt.kizby.app/icons/Kizby.icns"/> <solaris/> <win useIco="true"> <ico path="ca.mt.kizby.app/icons/Kizby.ico"/> <bmp/> </win> </launcher> My pom.xml then used the following to copy the icons into the place matching the locations .product file, keeping both PDE and Tycho happy. <build> <plugins> <!-- This seems necessary as the icons can't be found otherwise. Tycho seems to resolve the icons from the target/products/<product.id> directory whereas PDE seems to prefix with 'platform:/plugin' and then resolve it appropriately... --> <plugin> <artifactId>maven-antrun-plugin</artifactId> <version>1.6</version> <executions> <execution> <phase>process-resources</phase> <id>copy-icons</id> <configuration> <target> <echo message="Copying app icons into place" /> <!-- would be nicer if we could iterate on all the defined products ${project.build.directory}/products/${product.id} --> <copy todir="${project.build.directory}/products/ca.mt.kizby.product"> <!-- resolved relative to the project directory --> <fileset dir="../../kizby/plugins"> <include name="ca.mt.kizby.app/icons/**" /> </fileset> </copy> </target> </configuration> <goals> <goal>run</goal> </goals> </execution> </executions> </plugin>
(In reply to comment #49) > David: the .product icon attributes specify where the icon is found, not > where it should be placed. Unfortunately PDE/Build and Tycho differ in how > they resolve the icons: Tycho seems to resolve the icons from the > target/products/<product.id> directory. > Ok, and just to be real explicit, what does your kizby Info.plist end up looking like? Still _just_ icon name, I assume? <key>CFBundleIconFile</key> <string>Kizby.icns</string> I'm asking since p2 publisher is planning a change "not to change icon value if already supplied" (or, something like that) ... and all of our "pre-supplied" Info.plist files already say (just) <key>CFBundleIconFile</key> <string>Eclipse.icns</string> So ... just trying to confirm ahead of time that's correct. Thanks for your help.
(In reply to comment #49) > David: the .product icon attributes specify where the icon is found, not where > it should be placed. Unfortunately PDE/Build and Tycho differ in how they > resolve the icons: Tycho seems to resolve the icons from the > target/products/<product.id> directory. Actually, a correct path from the project root should be enough. Then, the icon files should be automatically copied to the target/products/<productId> folder (see [1]) [1] http://git.eclipse.org/c/tycho/org.eclipse.tycho.git/tree/tycho-p2/tycho-p2-publisher-plugin/src/main/java/org/eclipse/tycho/plugins/p2/publisher/PublishProductMojo.java#n148
(In reply to comment #50) > Ok, and just to be real explicit, what does your kizby Info.plist end up > looking like? Still _just_ icon name, I assume? > > <key>CFBundleIconFile</key> > <string>Kizby.icns</string> That's right, David. (In reply to comment #51) > Actually, a correct path from the project root should be enough. Then, the > icon files should be automatically copied to the target/products/<productId> > folder (see [1]) Ah, that's good news. I meant to but never did dig into it.
Created attachment 231319 [details] patch to 'copy up" the icns files in pom. I've a test build running, but ... timing is tight ... and I think this patch would fix the issue without the p2 publisher fix ... or, we could just wait and see if the p2 publisher fix fixes the issue? I think its due soon ... tonight? tomorrow? At any rate, the key part of the patch is I use the maven-resources-plugin during prepare-package phase to copy *.icns from ../../rt.equinox.framework/features/org.eclipse.equinox.executable.feature /bin/cocoa/macosx/x86/Eclipse.app/Contents/Resources to ${project.build.directory}/../ That's "one level above target". Alternatively, we could just make a physical copy of the icns file, if people thought that preferable.
I'll ask Ian to review, since it was his suggestion to "in the meantime, make sure the icon is located where it can be found". Suggestions welcome.
Comment on attachment 231319 [details] patch to 'copy up" the icns files in pom. FYI. Patch didn't work, in my test build. Not sure if I must messed up the formatting (trying to format it) But said it could not create /<full path/target/.. <outputDirectory>${project.build.directory}/../ </outputDirectory> There must be a better variable to use, anyway (I hate relative assumptions).
(In reply to comment #55) > Comment on attachment 231319 [details] > patch to 'copy up" the icns files in pom. > > FYI. Patch didn't work, in my test build. Not sure if I must messed up the > formatting (trying to format it) But said it could not create > /<full path/target/.. > > <outputDirectory>${project.build.directory}/../ > </outputDirectory> > > There must be a better variable to use, anyway (I hate relative assumptions). Turns out, <outputDirectory>${basedir}</outputDirectory> copies the icons to the right place, and in a test build, appears to work. That is, no "FileNotFoundExceptions" in log any more, and the Info.plist file seems correct, not only for the icon, but also "product id", etc. <key>CFBundleExecutable</key> <string>eclipse</string> <key>CFBundleGetInfoString</key> <string>Eclipse 4.3 for Mac OS X, Copyright IBM Corp. and others 2002, 2012. All rights reserved.</string> <key>CFBundleIconFile</key> <string>Eclipse.icns</string> <key>CFBundleIdentifier</key> <string>org.eclipse.sdk.ide</string> <key>CFBundleInfoDictionaryVersion</key> <string>6.0</string> <key>CFBundleName</key> <string>eclipse</string> <key>CFBundlePackageType</key> <string>APPL</string> <key>CFBundleShortVersionString</key> <string>4.3.0</string> <key>CFBundleSignature</key> <string>????</string> <key>CFBundleVersion</key> <string>4.3.0.I20130522-2226</string> <key>NSHighResolutionCapable</key> <true/> <key>CFBundleDevelopmentRegion</key> <string>English</string> Still need to update the copyright though? If our Info.plist "pre-filled in" version is being copied from the right place. It is currently being copied from rt.equinox.framework/bundles/org.eclipse.equinox.executable/bin/cocoa/macosx/x86_64 (or, rt.equinox.framework/bundles/org.eclipse.equinox.executable/bin/cocoa/macosx/x86
Created attachment 231341 [details] proposed patch to copy up icns Corrected to use ${basedir}
I've made this commit to master, so I can do a test build this morning. Assuming no problems or objections, will propose we respin for RC2. http://git.eclipse.org/c/platform/eclipse.platform.releng.aggregator.git/commit/?id=a47de7c69e8e755e00a58ee34b73c0c51f31ac05
(In reply to comment #57) > Created attachment 231341 [details] [diff] > proposed patch to copy up icns > > Corrected to use ${basedir} Is there some formatting involved? Just looking at the patch, it looks huge and very hard to review.
(In reply to comment #59) > (In reply to comment #57) > > Created attachment 231341 [details] [diff] > > proposed patch to copy up icns > > > > Corrected to use ${basedir} > > Is there some formatting involved? Just looking at the patch, it looks huge > and very hard to review. Yes, sorry, formatting is the only way I can read XML, but looking at the patch, it seems it doesn't handle space changes very well. And, there's a lot of repetition, In short, it adds one "maven-resources-plugin" to each of our 6 "products". 5 are identical, the 6th, for OSGI starter kit, is similar, but a little different. It might be easiest at this point to look at just one of those "maven-resources-plugin" clauses in the source tree: http://git.eclipse.org/c/platform/eclipse.platform.releng.aggregator.git/tree/eclipse.platform.releng.tychoeclipsebuilder/sdk/pom.xml#n44 And/or, to look at the results of the test build. It just finished, so I'll be taking a look myself. http://build.eclipse.org/eclipse/builds/4I/siteDir/eclipse/downloads/drops4/I20130523-0810/
I checked the SDK from test build, and it was correct. I believe in future we may want to discuss less "wordy" solutions ... might be easier just to have multiple copies of the icns file where we need them -- do more in a "product parent pom", or similar. But ... for now ... does what's needed, as far as I can tell. Please verify or reopen if anyone disagrees with this solution. (Or, if I got the wrong icon ... pretty sure its the most recent ... but ... parts of the Git repo are pretty messy, --- and my eye sight isn't what it once was :) ... so hard for me to tell).
Unfortunately I can't make sense of the patch itself, but I'll +1 it since it seems to fix the problem, and that's important at this point. The patch is related to build time artifacts, so if the build works (without errors) and the the icons are available, we can be reasonably certain that we haven't broken anything. We have a few more iterations to check things. (The most important thing is that our metadata still allows us to properly update). There are two main reasons we do code reviews during the RCs. The first one is to have someone double-check your work. Unfortunately I don't think any of us have enough experience with something as complicated as the Eclipse build to double check the technical details of the Tycho configuration. We will need to rely on manual testing for that. The second reason is to provide a sober second thought on our decisions. In this case, I wholeheartedly give this a +1.
Verified in I20130523-1400. The copyright date can be fixed with bug 408276.
I just wanted to be sure to acknowledge that I take "patch cleanliness" very seriously, and vow to do a better job in future. Not that its a great excuse, but if I had not had a 3 hour dental appointment that day I would have re-done it. So, appreciate your trust and patience, and as restitution :) I've opened bug 409076. I hope in future, we can have some "standard formatting" for POMs, and other XML, .... at least in aggregator ... I don't expect every project to use the same rules ... so I hope in future it is easier to make clean patches ... for ourselves, and and others that want to contribute to the the project.
