Community
Participate
Working Groups
As far as I can tell this feature, in the bundles directory, is not used (well, except for one place ... I'll need to patch "releng") and the new/accurate one that is used is rt.equinox.framework/features/org.eclipse.equinox.executable.feature Its very confusing having two the same name (both called org.eclipse.equinox.executable in the feature.xml Id). I ran into this trying to sort out the thread effecting bug 419092 and bug 420134. As far as I can tell, bundles/org.eclipse.equinox.executable is no longer used. I wouldn't delete it here during M3 warmup week ... but ... after that?
Created attachment 236944 [details] patch to remove "file system reference". As I mentioned, there is a place in "configuration features" that copies some files from the "old" location ... so this patch changes that to do the copy from the "new" location. As far as I know, this won't functionally improve anything ... just make searches, etc., easier, and easier to load the right one into workspace. I'm doing a local test build now. Assuming there's nothing that obviously breaks from this patch, I may go ahead and apply it for M3. Then you (equinox team) can remove at will (after M3).
(In reply to David Williams from comment #0) > As far as I can tell, bundles/org.eclipse.equinox.executable is no longer used. Compare the logs: feature: https://git.eclipse.org/c/equinox/rt.equinox.framework.git/log/features/org.eclipse.equinox.executable.feature bundle: https://git.eclipse.org/c/equinox/rt.equinox.framework.git/log/bundles/org.eclipse.equinox.executable It looks like the fact that the bundle was moved to be a feature was forgotten, and the real executable development is still happening in the bundle. This most probably works fine because binaries are stored in the repo, and the process of Eclipse assembly is correct (although sources don't match the binaries).
(In reply to Krzysztof Daniel from comment #2) > (In reply to David Williams from comment #0) > It looks like the fact that the bundle was moved to be a feature was > forgotten, and the real executable development is still happening in the > bundle. So ... I'm not only one confused? :) I've applied my patches for M3. I'll let Tom and Silenio and others decide the best to to resolve larger issue ... but it is very confusing the way it is.
Paul, I think you know why there is two. IIRC two was needed to allow PDE build to still work. I think that ship has sailed and we can remove the one that was used for PDE build, right?
I was not aware we had the feature bundle that duplicated the executable source code. We definitely must have just one copy of the source code. The most up to date code is in: https://git.eclipse.org/c/equinox/rt.equinox.framework.git/log/bundles/org.eclipse.equinox.executable So if that is the one that is going to be delete, we need to update the feature bundle.
For CBI we only build the feature now, http://git.eclipse.org/c/equinox/rt.equinox.framework.git/tree/pom.xml#n77 The bundles one is left over from when PDE build was building the executable feature. PW
So we need the executables updates to go into the feature version. PW
(In reply to Paul Webster from comment #7) > So we need the executables updates to go into the feature version. > > PW Selinio, I think we need to do this very soon. We are starting to get contributions from Gerrit against the source code from the features project (see https://git.eclipse.org/r/#/c/17874/) while other contributions are against the bundles project (see https://git.eclipse.org/r/#/c/17837/).
Yes, I agree. Arun, please work on this after M3 is done and attach a patch for review.
Friendly reminder ... this would be a good week to remove this project from master (and Kepler) ... just to give time to revert if unexpected implications. (Of course, you may have higher priories ... so feel free to balance all those with this one).
For what its worth, I removed the "bundles feature" from my working test repo and the build still succeeded. That's not to say "it works" :) ... but, just that there is no hidden dependencies. I realize the "big part" of this work is to get swt/launcher team to update source in the right place. Just trying to be helpful.
(In reply to David Williams from comment #11) > For what its worth, I removed the "bundles feature" from my working test > repo and the build still succeeded. That's not to say "it works" :) ... but, > just that there is no hidden dependencies. I realize the "big part" of this > work is to get swt/launcher team to update source in the right place. > > Just trying to be helpful. David, you speak about changes in rt.equinox.framework repository only or something must happen in swt repos too?
(In reply to Alexander Kurtakov from comment #12) > (In reply to David Williams from comment #11) > > For what its worth, I removed the "bundles feature" from my working test > > repo and the build still succeeded. That's not to say "it works" :) ... but, > > just that there is no hidden dependencies. I realize the "big part" of this > > work is to get swt/launcher team to update source in the right place. > > > > Just trying to be helpful. > > David, you speak about changes in rt.equinox.framework repository only or > something must happen in swt repos too? No, just rt.equinox.framework ... I said "swt/launcher team" just because its the same people (er, right?)
(In reply to David Williams from comment #13) > (In reply to Alexander Kurtakov from comment #12) > > (In reply to David Williams from comment #11) > > > For what its worth, I removed the "bundles feature" from my working test > > > repo and the build still succeeded. That's not to say "it works" :) ... but, > > > just that there is no hidden dependencies. I realize the "big part" of this > > > work is to get swt/launcher team to update source in the right place. > > > > > > Just trying to be helpful. > > > > David, you speak about changes in rt.equinox.framework repository only or > > something must happen in swt repos too? > > No, just rt.equinox.framework ... I said "swt/launcher team" just because > its the same people (er, right?) I guess it's just me that is only swt that's why I asked whether smth is needed there.
I have started working on creating a patch for this but realized it is more complicated than I initially thought it would be, may be because I haven't worked on this code before and so don't have the necessary background. I have a few questions which might sound dumb but I'll ask them anyway :) - For source as well as resource files, there are many changes which have gone into either the feature bundle only or the bundles version only and some of the files look very different. In these cases, should I just try to merge code from both the copies and create a combined file with all the changes? I understand that may not make sense in all scenarios but I'm just trying to think aloud about a good merge strategy here... - There are a few files like customBuildCallbacks.xml, target.build.properties and target.build.xml which are present in the bundles version but not in the feature version. I'm assuming they are not needed by the feature bundle anymore and can be safely deleted, is that correct? - In the current scenario, whenever the launcher binaries are updated, a corresponding 'binaryTag' value is updated in the bundles version of the build.properties file but this value is not present in the feature bundle at all. What is the role played by this variable and is it needed at all? If it is indeed needed, how are things working fine even though it is not present in the feature version? As an aside here, the build process also needs to be updated then to commit the binaryTag value to the appropriate file as needed. I will continue working on the patch and will come back with more questions if needed. Thanks in advance for any guidance!
(In reply to Arun Thondapu from comment #15) > I have started working on creating a patch for this but realized it is more > complicated than I initially thought it would be, may be because I haven't > worked on this code before and so don't have the necessary background. I > have a few questions which might sound dumb but I'll ask them anyway :) Well, I'll give a few dumb answers which I'm sure are half right and half wrong ... and maybe given that you can figure out which are which :) > - For source as well as resource files, there are many changes which have > gone into either the feature bundle only or the bundles version only and > some of the files look very different. In these cases, should I just try to > merge code from both the copies and create a combined file with all the > changes? I understand that may not make sense in all scenarios but I'm just > trying to think aloud about a good merge strategy here... According to comment 5, <quote> The most up to date code is in: https://git.eclipse.org/c/equinox/rt.equinox.framework.git/log/bundles/org.eclipse.equinox.executable So if that is the one that is going to be delete, we need to update the feature bundle.</quote> This sounds to me like a "merge" is not really needed, ... one way or another code the "up to date" bundle needs to "replace" the (source) code that's in the feature/ version. > > - There are a few files like customBuildCallbacks.xml, > target.build.properties and target.build.xml which are present in the > bundles version but not in the feature version. I'm assuming they are not > needed by the feature bundle anymore and can be safely deleted, is that > correct? Correct. Well, I'm half guessing, but definitely the customBuildCallbacks.xml would no longer be needed (that is a definite "PDE Build" file) ... I don't know enough to know if target.build.properties and target.build.xml are used to produce the actual "executables". I'd do a search to see where they are called from ... and if not obviously called from anywhere, safe to remove them. > - In the current scenario, whenever the launcher binaries are updated, a > corresponding 'binaryTag' value is updated in the bundles version of the > build.properties file but this value is not present in the feature bundle at > all. What is the role played by this variable and is it needed at all? If it > is indeed needed, how are things working fine even though it is not present > in the feature version? As an aside here, the build process also needs to be > updated then to commit the binaryTag value to the appropriate file as needed. My guess is not longer needed ... unless ... does the 'binaryTag' end up as part of the executable name? (internal field or literally as part of the name)? If so, probably still need, if not, probably not. If I may suggest the "key" to making this transition is for you to do what ever "external" process you do to do build the source (using the /feature version instead of the /bundle version ... And as problems and differences show up you'll know that you need and don't need. I never realized this would be so complicated ... and that things apparently have not been communicated well ... good thing we've discovered it now. Thanks for digging into it!
(In reply to Arun Thondapu from comment #15) > - For source as well as resource files, there are many changes which have > gone into either the feature bundle only or the bundles version only and > some of the files look very different. In these cases, should I just try to > merge code from both the copies and create a combined file with all the > changes? I understand that may not make sense in all scenarios but I'm just > trying to think aloud about a good merge strategy here... I think that the proper way of dealing with changes is not really about merge, as development was happening in both places. What really needs to be done, is a rewind to the fork root, and then applying patches from both places, one by one, in a chronological order.
I believe all you need to do is to copy all the files from bundles/org.eclipse.equinox.executable/library into features/org.eclipse.equinox.executable.feature/library The files customBuildCallbacks.xml, target.build.properties and target.build.xml are no longer needed. They were used by the p2 build. The binaryTag is no longer needed. They were necessary when the rt.equinox.binaries repository was not branched for maintenance builds. The p2 build file customBuildCallbacks.xml would copy that specific tag from the binaries repo into the src repo. This is now done by branch: R3_8_maintenance, R3_9_maintenance and master. Note that the releng script (org.eclipse.equinox.launcher.releng/build.xml) should stop updating it as well. Finally, the releng script needs to be updated to point the "features/org.eclipse.equinox.executable.feature" instead of "bundles/org.eclipse.equinox.executable". I do not think there are any changes needed in the Hudson jobs.
Created attachment 237879 [details] Proposed patch
(In reply to Silenio Quarti from comment #18) > I believe all you need to do is to copy all the files from > > bundles/org.eclipse.equinox.executable/library > > into > > features/org.eclipse.equinox.executable.feature/library > > The files customBuildCallbacks.xml, target.build.properties and > target.build.xml are no longer needed. They were used by the p2 build. > > The binaryTag is no longer needed. They were necessary when the > rt.equinox.binaries repository was not branched for maintenance builds. The > p2 build file customBuildCallbacks.xml would copy that specific tag from the > binaries repo into the src repo. This is now done by branch: > R3_8_maintenance, R3_9_maintenance and master. Note that the releng script > (org.eclipse.equinox.launcher.releng/build.xml) should stop updating it as > well. > > Finally, the releng script needs to be updated to point the > "features/org.eclipse.equinox.executable.feature" instead of > "bundles/org.eclipse.equinox.executable". > > I do not think there are any changes needed in the Hudson jobs. Thanks for pointing out the specific changes that need to be done Silenio! I have attached a patch above which includes all the changes that are necessary. Kindly review and provide your feedback. Thanks!
(In reply to Arun Thondapu from comment #20) > Thanks for pointing out the specific changes that need to be done Silenio! > > I have attached a patch above which includes all the changes that are > necessary. Kindly review and provide your feedback. Thanks! After I apply the patch, there are still some files left in bundles/org.eclipse.equinox.executable (see below). Is that intentional? I thought "bundles/org.eclipse.equinox.executable" would be removed completely. bundles/org.eclipse.equinox.executable/ bundles/org.eclipse.equinox.executable/bin bundles/org.eclipse.equinox.executable/bin/carbon bundles/org.eclipse.equinox.executable/bin/carbon/macosx bundles/org.eclipse.equinox.executable/bin/carbon/macosx/ppc bundles/org.eclipse.equinox.executable/bin/carbon/macosx/ppc/Eclipse.app bundles/org.eclipse.equinox.executable/bin/carbon/macosx/ppc/Eclipse.app/Contents bundles/org.eclipse.equinox.executable/bin/carbon/macosx/ppc/Eclipse.app/Contents/Resources bundles/org.eclipse.equinox.executable/bin/carbon/macosx/ppc/Eclipse.app/Contents/Resources/Eclipse.icns bundles/org.eclipse.equinox.executable/bin/carbon/macosx/x86 bundles/org.eclipse.equinox.executable/bin/carbon/macosx/x86/Eclipse.app bundles/org.eclipse.equinox.executable/bin/carbon/macosx/x86/Eclipse.app/Contents bundles/org.eclipse.equinox.executable/bin/carbon/macosx/x86/Eclipse.app/Contents/Resources bundles/org.eclipse.equinox.executable/bin/carbon/macosx/x86/Eclipse.app/Contents/Resources/Eclipse.icns bundles/org.eclipse.equinox.executable/bin/cocoa bundles/org.eclipse.equinox.executable/bin/cocoa/macosx bundles/org.eclipse.equinox.executable/bin/cocoa/macosx/ppc bundles/org.eclipse.equinox.executable/bin/cocoa/macosx/ppc/Eclipse.app bundles/org.eclipse.equinox.executable/bin/cocoa/macosx/ppc/Eclipse.app/Contents bundles/org.eclipse.equinox.executable/bin/cocoa/macosx/ppc/Eclipse.app/Contents/Resources bundles/org.eclipse.equinox.executable/bin/cocoa/macosx/ppc/Eclipse.app/Contents/Resources/Eclipse.icns bundles/org.eclipse.equinox.executable/bin/cocoa/macosx/x86 bundles/org.eclipse.equinox.executable/bin/cocoa/macosx/x86/Eclipse.app bundles/org.eclipse.equinox.executable/bin/cocoa/macosx/x86/Eclipse.app/Contents bundles/org.eclipse.equinox.executable/bin/cocoa/macosx/x86/Eclipse.app/Contents/Resources bundles/org.eclipse.equinox.executable/bin/cocoa/macosx/x86/Eclipse.app/Contents/Resources/Eclipse.icns bundles/org.eclipse.equinox.executable/bin/cocoa/macosx/x86_64 bundles/org.eclipse.equinox.executable/bin/cocoa/macosx/x86_64/Eclipse.app bundles/org.eclipse.equinox.executable/bin/cocoa/macosx/x86_64/Eclipse.app/Contents bundles/org.eclipse.equinox.executable/bin/cocoa/macosx/x86_64/Eclipse.app/Contents/Resources bundles/org.eclipse.equinox.executable/bin/cocoa/macosx/x86_64/Eclipse.app/Contents/Resources/Eclipse.icns bundles/org.eclipse.equinox.executable/bin/motif bundles/org.eclipse.equinox.executable/bin/motif/linux bundles/org.eclipse.equinox.executable/bin/motif/linux/x86 bundles/org.eclipse.equinox.executable/bin/motif/linux/x86/libXm.so.2 bundles/org.eclipse.equinox.executable/eclipse_update_120.jpg bundles/org.eclipse.equinox.executable/library bundles/org.eclipse.equinox.executable/library/win32 bundles/org.eclipse.equinox.executable/library/win32/eclipse.ico bundles/org.eclipse.equinox.executable/library/wpf bundles/org.eclipse.equinox.executable/library/wpf/eclipse.ico bundles/org.eclipse.equinox.executable/motif_root bundles/org.eclipse.equinox.executable/motif_root/about_files bundles/org.eclipse.equinox.executable/motif_root/about_files/mlpl-v10.html
Created attachment 238120 [details] Patch without deleted bundle files
(In reply to Silenio Quarti from comment #21) > (In reply to Arun Thondapu from comment #20) > > Thanks for pointing out the specific changes that need to be done Silenio! > > > > I have attached a patch above which includes all the changes that are > > necessary. Kindly review and provide your feedback. Thanks! > > After I apply the patch, there are still some files left in > bundles/org.eclipse.equinox.executable (see below). Is that intentional? I > thought "bundles/org.eclipse.equinox.executable" would be removed > completely. > Thanks for the review Silenio! The original patch I had submitted does indeed remove the "bundles/org.eclipse.equinox.executable" project entirely. However, after some investigation with the patch, I found out that it is not applying cleanly on top of HEAD on the rt.equinox.framework repo and this seems to be because git has some issues with applying patches which include binary file diffs/changes. I got multiple errors like the one below when I tried 'git apply' from cmdline with the original patch: error: cannot apply binary patch to 'bundles/org.eclipse.equinox.executable/bin/carbon/macosx/ppc/Eclipse.app/Contents/Resources/Eclipse.icns' without full index line error: bundles/org.eclipse.equinox.executable/bin/carbon/macosx/ppc/Eclipse.app/Contents/Resources/Eclipse.icns: patch does not apply This could be the reason the patch would not have applied properly in your workspace, causing the mentioned files to be not deleted. To workaround this problem, I have now submitted a new patch which includes only the code changes that are made when files are copied over from bundles/org.eclipse.equinox.executable/library to features/org.eclipse.equinox.executable.feature/library and also the changes needed in org.eclipse.equinox.launcher.releng/build.xml. After applying this patch, you can just delete bundles/org.eclipse.equinox.executable from the repo in your workspace and commit all the changes. Does this approach sound okay for you?
Thanks Arun, I have committed your patch (and deleted the bundle dir): http://git.eclipse.org/c/equinox/rt.equinox.framework.git/commit/?id=72c26472b6269957b427fbc4b5251d5f3d047745
(In reply to Silenio Quarti from comment #24) > Thanks Arun, I have committed your patch (and deleted the bundle dir): > > http://git.eclipse.org/c/equinox/rt.equinox.framework.git/commit/ > ?id=72c26472b6269957b427fbc4b5251d5f3d047745 If it helps, I did a local test build after this change ... and nothing broke! (Well ... I mean the build finished without error :) ... guess it'll deserve some extra testing during "stabilization week" to make sure we are building what's expected, that source matches, etc.) Much thanks!
Should this be marked as fixed now for M4?
(In reply to Thomas Watson from comment #26) > Should this be marked as fixed now for M4? I think so (though, the "equinox launcher team" should decide if should be back ported to R4_3 maintenance now ... I just mention that since doing so would prevent accidentally "using" it in case long term maintenance required ... but, for me, fixing in master is the main thing).
Fixed for M4. Opened bug 423840 to consider for Kepler SR2.
