Bug 205833 - Export missing Product.exe
Summary: Export missing Product.exe
Status: RESOLVED FIXED
Alias: None
Product: PDE
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.3.1   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.4 M5   Edit
Assignee: Benjamin Cabé CLA
QA Contact:
URL:
Whiteboard:
Keywords: bugday, contributed
Depends on: 205832
Blocks:
  Show dependency tree
 
Reported: 2007-10-09 13:20 EDT by Andrew Niefer CLA
Modified: 2008-02-04 14:54 EST (History)
4 users (show)

See Also:
baumanbr: review+


Attachments
mylyn/context/zip (761 bytes, application/octet-stream)
2008-01-31 05:41 EST, Benjamin Cabé CLA
no flags Details
The launcher path's is retrieved in eclipse.launch system property (1.33 KB, patch)
2008-01-31 05:45 EST, Benjamin Cabé CLA
no flags Details | Diff
Updated patch... (2.73 KB, patch)
2008-01-31 12:40 EST, Benjamin Cabé CLA
no flags Details | Diff
org.eclipse.pde.ui.patch (4.54 KB, patch)
2008-02-04 11:27 EST, Chris Aniszczyk CLA
no flags Details | Diff
mylyn/context/zip (1.40 KB, application/octet-stream)
2008-02-04 11:27 EST, Chris Aniszczyk CLA
no flags Details
org.eclipse.pde.patch (4.77 KB, patch)
2008-02-04 14:38 EST, Chris Aniszczyk CLA
no flags Details | Diff
mylyn/context/zip (1.58 KB, application/octet-stream)
2008-02-04 14:38 EST, Chris Aniszczyk CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Niefer CLA 2007-10-09 13:20:18 EDT
When exporting a product without the delta-pack, the eclipse.exe is taken from the running eclipse.

However, if the running eclipse is a branded product the launcher is not named eclipse.exe and does not get included in the product.

Export should try and included the branded exe
Comment 1 Brian Bauman CLA 2007-11-12 15:18:21 EST
We should be able to find the name of the executable using the eclipse.launcher property as specified in bug 205832.
Comment 2 Chris Aniszczyk CLA 2007-11-12 15:21:43 EST
oooo! we should get this in for 3.4
Comment 3 Benjamin Cabé CLA 2008-01-31 05:41:41 EST
Created attachment 88399 [details]
mylyn/context/zip

Mylyn context
Comment 4 Benjamin Cabé CLA 2008-01-31 05:45:21 EST
Created attachment 88400 [details]
The launcher path's is retrieved in eclipse.launch system property

It should work, even if I haven't fully tested it!
Comment 5 Chris Aniszczyk CLA 2008-01-31 09:56:50 EST
Thanks Ben, we'll look at it for 3.4M5
Comment 6 Andrew Niefer CLA 2008-01-31 11:23:47 EST
The property "eclipse.launcher" is set by Main when the command line argument "-launcher" is specified.  This argument is specified automatically by the eclipse launcher.  It also seems to be specified automatically by PDE when debugging.

However, if Eclipse was started using java.exe without the -launcher argument then  the eclipse.launcher property will not be set.  PDE should fall back and look for eclipse.exe in this case.  It looks like the current patch may throw an NPE in this case.
Comment 7 Benjamin Cabé CLA 2008-01-31 11:38:53 EST
Oops.
What would be the correct behaviour to your mind ? If eclipse.launcher is here, let's use it; else, keep the current behaviour ?



(In reply to comment #6)
> The property "eclipse.launcher" is set by Main when the command line argument
> "-launcher" is specified.  This argument is specified automatically by the
> eclipse launcher.  It also seems to be specified automatically by PDE when
> debugging.
> 
> However, if Eclipse was started using java.exe without the -launcher argument
> then  the eclipse.launcher property will not be set.  PDE should fall back and
> look for eclipse.exe in this case.  It looks like the current patch may throw
> an NPE in this case.
> 

Comment 8 Chris Aniszczyk CLA 2008-01-31 11:46:23 EST
That seems reasonable, new patch ;p?
Comment 9 Benjamin Cabé CLA 2008-01-31 11:51:05 EST
OK... ;) 
Let's go!

Benjamin, a.k.a. the trivial patcher :)

(In reply to comment #8)
> That seems reasonable, new patch ;p?
> 

Comment 10 Chris Aniszczyk CLA 2008-01-31 11:55:09 EST
(In reply to comment #9)
> Benjamin, a.k.a. the trivial patcher :)

Anything to help things not fall through the cracks is greatly appreciated. We can get you on the PDE Contributers page :P (http://www.eclipse.org/pde/pde-ui/committers/committers.php)
Comment 11 Benjamin Cabé CLA 2008-01-31 12:40:52 EST
Created attachment 88431 [details]
Updated patch...

...with a bit of code cleaning :)
Comment 12 Chris Aniszczyk CLA 2008-02-04 11:27:22 EST
Created attachment 88787 [details]
org.eclipse.pde.ui.patch

An updated patch. I found an issue when you're self-hosting... we don't properly pass the -launcher when we are on win32 causing an issue when you're actually going to test this stuff in self-hosting mode ;)
Comment 13 Chris Aniszczyk CLA 2008-02-04 11:27:25 EST
Created attachment 88788 [details]
mylyn/context/zip
Comment 14 Brian Bauman CLA 2008-02-04 14:08:26 EST
The patch looks good to me :)  

Chris and I were talking and the only thing that might make this a little more robust is to check to make sure the File specified in the system properties doesn't only exist, but is also not a directory (which is why Chris had to add the launcher modification).
Comment 15 Chris Aniszczyk CLA 2008-02-04 14:37:09 EST
yap!
Comment 16 Chris Aniszczyk CLA 2008-02-04 14:38:02 EST
Created attachment 88814 [details]
org.eclipse.pde.patch

I updated the patch to be a bit more resilient if we hit a corner case of getting a bad launcher path.

Thanks to everyone for their help on this issue.
Comment 17 Chris Aniszczyk CLA 2008-02-04 14:38:04 EST
Created attachment 88815 [details]
mylyn/context/zip
Comment 18 Chris Aniszczyk CLA 2008-02-04 14:38:17 EST
done!
Comment 19 Benjamin Cabé CLA 2008-02-04 14:54:00 EST
great :)