Comment 1 Kaloyan Raev 2010-05-28 04:48:11 EDT

This bug is observed in WTP 3.2 RC3

Comment 2 Angel Vera 2010-05-28 10:38:24 EDT

This sounds like a change in the platform, sending over to the P2 team for comments. I am not sure if the change in bug# 282333 affected this behaviour

Comment 3 Pascal Rapicault 2010-05-28 12:15:23 EDT

Is the Server Discovery code using the p2 discovery code?
Which adapter were you installing?

Comment 4 Angel Vera 2010-05-28 14:04:15 EDT

Not sure what you mean by the p2 discovery so I would say, we are probably not using it.

We have a list of repositories and we query each one of them manually.

Comment 5 Pascal Rapicault 2010-05-28 14:11:00 EDT

So how do you think it is related to p2?
How do you perform the installation?

Comment 6 Steffen Pingel 2010-05-28 14:15:11 EDT

p2 discovery relies on the platform default for the restart/prompting behavior. 

AFAIK webtools has their own P2 based discovery style implementation for managing server adapters so if there is a regression it's more likely to be in the framework or webtools.

Comment 7 Angel Vera 2010-05-28 14:34:53 EDT

(In reply to comment #5)

We do the installation inside of our Extension.install(...) by executing: engine.perform(plan, PhaseSetFactory.createDefaultPhaseSet(), monitor);

The plan is retrieved from from a IPlanner and creating a changeRequest using the profile and adding the IInstallableUnit.

Comment 8 Angel Vera 2010-05-28 14:42:26 EDT

(In reply to comment #6)
> AFAIK webtools has their own P2 based discovery style implementation for
> managing server adapters so if there is a regression it's more likely to be in
> the framework or webtools.

We have our own P2 based discovery style, we haven't changed since M7 was shipped, but I can't say with confidence that the restart dialog was there. I will have to go back and try.

Which framework are you referring to?

The reason why I thought this might be caused by a change in P2 it was based on the fact that bug 285197, reported at some point having to eliminate our restart dialog, because p2 was now providing one. I also discovered bug 282333, which seemed to be related to something related to startup.. but I didn't dig too much into that bug. I went based on when were the changes released. 

I am still very suspicious of bug 285197, and if we re-introduce the restart dialog, what are the changes that at some point the restart dialog will show up again twice.

Comment 9 Steffen Pingel 2010-05-28 16:02:52 EDT

The policy is controlled by setting ProfileModificationJob.restartPolicy. I would recommend setting a break point in the setter to see if there is something in wtp changing that.

Comment 10 Angel Vera 2010-05-28 16:58:00 EDT

Created attachment 170432 [details]
partial p2.engine trace (possibly useless)

Comment 11 Angel Vera 2010-05-28 16:58:32 EDT

I put a breakpoint at: 

ProfileModificationJob [entry] - ProfileModificationJob(String, ProvisioningSession, String, IProvisioningPlan, ProvisioningContext)	
ProfileModificationJob [entry] - getRestartPolicy()	
ProfileModificationJob [entry] - setRestartPolicy(int)	

And at no point in time anything got hit. I also enabled the p2.engine trace in hope that it will shed some light.. but I can't read it. Unfortunatly I didn't catch everything since I forgot to increased my console buffer.. if you really need a complete trace I can provide one.

Comment 12 Pascal Rapicault 2010-05-28 20:19:00 EDT

Could you please provide detailed steps to reproduce. Just so I'm sure I'm looking at what is necessary.
Also could you indicate where your code is.

Comment 13 Angel Vera 2010-05-31 14:20:57 EDT

(In reply to comment #12)

CVS: :pserver:anonymous@dev.eclipse.org:/cvsroot/webtools
paths: servertools/plugins/org.eclipse.wst.server.discovery

To reproduce the problem you will need WTP 3.2 RC3: 
Download the first five zip + "Traditional Zip Files for Web Tools Platform"  from: 
 http://build.eclipse.org/webtools/committers/wtp-R3.2.0-S/20100529234347/S-3.2.0RC3-20100529234347/

Step to reproduce the problem:
1) Open the Servers view 
2) Right click on the servers view: New > Server
3) Click on the link shown in the right top hand of the wizard that comes up: "Download additional server adapters"
4) You can select "Jonas WTP Adapter" or the "Jetty Generic Server Adaptor" as those are the simplest ready to install adapters.
5) "Next" > "I accept the terms.." > Finish 
6) There will be two prompts press "OK" for both
7) At the end the progress monitor disapears and there is no prompt to restart, But everything was downloaded and installed, if you restart eclipse the new server adapter is there.

Comment 14 Thomas Watson 2010-06-02 11:13:15 EDT

RC3 is done.  Moving to RC4 but this likely will need to be deferred.

Comment 15 Susan McCourt 2010-06-02 13:13:50 EDT

I suspect you haven't had a restart dialog since M5.
The restart behavior changed in bug 274876.  The issues are summarized in bug 274876 comment 14.

There used to be a "free restart" that occurred any time the self profile changed, but this has caused problems because there are cases where a restart should not happen.  The operations API (introduced in M5) consolidates all of the restart behavior, including configurable restart behavior.

I suspect that your code is not using operations at all and that you are creating your own plan and running it in the engine.  In this case, you'll need to manage the restart behavior yourself.  

At this point in the release, the simplest way to do this is to use the internal class ProvisioningOperationRunner.  Assuming that your provisioning work is done in a job, you can call:

ProvisioningOperationRunner.manageJob(job, ProvisioningJob.RESTART_OR_APPLY);

This will set up the listeners on the job and call the restart code if the job finishes successfully.

Comment 16 Angel Vera 2010-06-03 15:00:03 EDT

(In reply to comment #15)
Given your comments I am taking this bug back then. Thank you for the insights, we will provide a fix from our side.

Comment 17 Angel Vera 2010-06-03 15:44:16 EDT

Created attachment 171011 [details]
Pre-install restart warning

Given that:
-----------
- WTP is on its final stretch of RC4
- The bug has a workaround
- The bug doesn't break the function
- The scenario of downloading and installing new server adapter is not a common required action and it is most often only done once or twice during the usage of the product.
- There is already a message dialog that warns the user about restarting the product after the installation is completed


My proposed approach for this bug is,

For 3.2.1:
---------
Provide a proper fix that displays a restart dialog at the end of the installation.

For 3.2 RC4:
------------
Is to only change the message that shows up in the dialog that warns the user about restarting. The change to the message is to make it explicit to the user that he needs to restart the workbench after the download and install is completed. 

I have attached an image of the dialog that will be change, this dialog shows after the user has selected a Server Adapter but before the download/install process is started.

Comment 18 Angel Vera 2010-06-03 16:29:17 EDT

I have opened Bug# 315662 to address the message changing into 3.2

Comment 19 Kaloyan Raev 2010-07-07 07:29:27 EDT

Angel, I hope we won't forget fixing this bug for 3.2.1.

Comment 20 Angel Vera 2010-07-07 11:24:53 EDT

* Explain why you believe this is a stop-ship defect. Or, if it is a "hotbug" (requested by an adopter) please document it as such. 
This is an important bug for adopters with discoverable servers.

* Is there a work-around? If so, why do you believe the work-around is insufficient? 
This fix was deferred from 3.2, because we didn't feel comfortable making changes that late in the cycle. So we put in a workaround for 3.2 that involved just a text chance. In 3.2.1 we are reverting the text as it used to be, and implementing the fix that the p2 team has recommended.

* How has the fix been tested? Is there a test case attached to the bugzilla record? Has a JUnit Test been added? 
- Run through downloading a few different server adapters, 
- Ensure that the changed text displayed correctly, 
- Ensure that a restart prompt came up at the end of the installation,
- Ensure that the adapter was downloaded correctly

* Give a brief technical overview. Who has reviewed this fix? 
The fix is to use the P2 API to add a JobChangeListener to the install job, so that when the install job is done a restart dialog will show.

* What is the risk associated with this fix?
low

Comment 21 Angel Vera 2010-07-07 13:58:15 EDT

Created attachment 173688 [details]
v1.0

Comment 22 Angel Vera 2010-07-07 23:32:00 EDT

Changes committed and released to the 32M

Comment 23 Angel Vera 2010-07-07 23:34:19 EDT

Changes committed and released to HEAD

Comment 24 Kaloyan Raev 2010-07-08 13:26:46 EDT

I confirm this works with the latest I-build. Thanks!

I guess we can close this bug now.

Comment 25 Angel Vera 2010-07-08 14:04:58 EDT

Thanks Kayolan

Comment 26 Eclipse Genie 2017-10-11 16:34:04 EDT

New Gerrit change created: https://git.eclipse.org/r/108969