Bug 71515

Since the actions like Build etc. work so poorly on groups due to this issue,
and since this kind of functionality is usually better invoked from a parent
project anyway, I am removing these actions from the groups module:

committed   * Up-To-Date  1.9         contrib/poorMansPortfolios/manifest.mf
committed   * Up-To-Date  1.13       
contrib/poorMansPortfolios/src/org/netbeans/modules/project/portfolios/PortfoliosTopComponent.java

jesse, is this still relevant?

Still a valid enhancement, though priority is I think lower.

what would be the currently relevant api clients?

I can't think of any current potential users. I thought there was something else that needed it, but I just grepped for
"71515" and did not find anything.

Change of default owner.

Needed for a proper fix of #185429 (see a171a47f3b25).

Based on my fix of bug 185429 I'd suggest to create a static method:

pu. st. void invokeActionSynchronously(ActionProvider ap, String cmd, Lookup ctx)

such method would do the same think I did - e.g. create a ProxyLookup with ActionEvent("waitFinished"). The method should detect that the provider does not support synchronous execution (getSource()[0] not changed) and in such case log a warning with identification of the provider.

+1 for Coherence module

Will see if I can do this soon.

Created attachment 116638 [details]
Start of patch with ActionProvider2

A cleaner API going forward - builds blocking nature directly into the SPI, and elininates the awkwardness of three separate methods when just one would suffice - but cumbersome to convert existing ActionProvider implementations due to the different control flow.

I'm reaching the conclusion that Future is almost always evil;  it says to the caller "please!  block on me!".  Almost any place that you have the signature

public Future<T> doSomething() {
}

you could replace it with something like

public void doSomething (Callback<T> callback);
static abstract class Callback<T> {
   void callback(T val);
   <E extends Exception> void onFail(E thrown) throws E { throw thrown; }
}

(parameterizing on an exception is a little weird, but it allows the same code to handle rethrow of runtime and checked exceptions without special-cases for each).


While you can still write a Callback which could, say, call this.notifyAll(), and the caller could block on that, an API like that doesn't give the impression that the natural way to use it is to block on it.

Or at the least, wrap all Futures in a wrapper instance which will throw an exception if waited on in the event thread.

I am already working on an alternate API involving a callback/listener of a kind. Though at first blush this seems like it would be less work to use in an existing ActionProvider.invokeAction implementation, the situation is complicated by the fact that many such implementations reschedule their work into a request processor, yet it is necessary to call a start method on the listener within the dynamic scope of the method... so while easier than returning Callable it is not trivial. Will post a new patch when I have it working.

(In reply to comment #7)
> Needed for a proper fix of #185429 (see a171a47f3b25).

Actually 43e85fce76d0 (bug #198275) removed the call site of this, so the ActionEvent trick was dead code; will just replace it with the standard API.

Created attachment 116685 [details]
Rewritten patch based on ActionProgress

Please review the revised patch. The SPI seems fairly straightforward to support from existing ActionProvider implementations.

Since the only API caller so far is the Coherence module, which I probably cannot even test, I will work on implementations of bug #206208 and/or bug #64991 to verify that everything works together naturally.

Would it be possible to smuggle in an indication that the action has not been executed at all? Eg. when you call Run/Debug/Profile on a class without a valid main method the BaseActionProvider actually skips the execution and displays a warning about missing main method. It would be great if an invoker of AP.invokeAction() could get back the information that the action was not, in fact, executed.

Created attachment 116689 [details]
Fix of bug #64991 using this API

(In reply to comment #17)
> Would it be possible to smuggle in an indication that the action has not been
> executed at all?

That is what the caller may infer when started() is never called. (Or else the provider just does not support #71515 yet; there is no way to differentiate those cases.)

> when you call Run/Debug/Profile on a class without a valid
> main method the BaseActionProvider actually skips the execution and displays a
> warning about missing main method.

Right. In the current patch in this circumstance I think BAP will call started() followed by finished(false), but the Javadoc permits it to call neither, which would probably be simpler to implement... so if that is useful to you I will do it that way.

BTW the patch for #64991 does not work to enable Project [Main] Project on a multiselection. Not sure if that is due to the use of ProjectActionPerformer, which cannot deal with multiselections. Anyway this is not a serious use case; the point is to be able to use Test 2 Projects, Generate Javadoc for 2 Projects, Clean & Build 2 Projects, etc., which all work.

(In reply to comment #16)
> Since the only API caller so far is the Coherence module, which I probably
> cannot even test, 

The patch looks well from Coherence point of view. Thanks Jesse.

(In reply to comment #19)
> (In reply to comment #17)
> > Would it be possible to smuggle in an indication that the action has not been
> > executed at all?
> 
> That is what the caller may infer when started() is never called. (Or else the
> provider just does not support #71515 yet; there is no way to differentiate
> those cases.)

What I need is to get a notification that the action is actually to be executed so I can initialize the profiler infrastructure. However, I think I might be able to infer such situation from the already available information.

> 
> > when you call Run/Debug/Profile on a class without a valid
> > main method the BaseActionProvider actually skips the execution and displays a
> > warning about missing main method.
> 
> Right. In the current patch in this circumstance I think BAP will call
> started() followed by finished(false), but the Javadoc permits it to call
> neither, which would probably be simpler to implement... so if that is useful
> to you I will do it that way.
> 
> BTW the patch for #64991 does not work to enable Project [Main] Project on a
> multiselection. Not sure if that is due to the use of ProjectActionPerformer,
> which cannot deal with multiselections. Anyway this is not a serious use case;
> the point is to be able to use Test 2 Projects, Generate Javadoc for 2
> Projects, Clean & Build 2 Projects, etc., which all work.

Profiling multiple projects doesn't make sense either so I am fine with it. But what I find missing is the integration of ActionProgress into Rerun action in ant based projects. Currently, if you re-run an ant based action you will not receive any notifications, right?

(In reply to comment #21)
> what I find missing is the integration of ActionProgress into Rerun action in
> ant based projects. Currently, if you re-run an ant based action you will not
> receive any notifications, right?

Right, because this action was not launched by the caller, so there is no reason to send notifications about it. ActionProgress is simply a workaround for the fact that ActionProvider.invokeAction as originally designed does not block or return Future. Once the original action has completed, it is irrelevant whether the user chooses to rerun some Ant target later.

I am not really sure what your use case is, but I do not think this API is what you are looking for. Anyway I thought you were using #206196 to run profiler infrastructure?

(In reply to comment #22)

Probably this will be a bit off-topic WRT the API review but anyway
> (In reply to comment #21)
> > what I find missing is the integration of ActionProgress into Rerun action in
> > ant based projects. Currently, if you re-run an ant based action you will not
> > receive any notifications, right?
> 
> Right, because this action was not launched by the caller, so there is no
> reason to send notifications about it. ActionProgress is simply a workaround
> for the fact that ActionProvider.invokeAction as originally designed does not
> block or return Future. Once the original action has completed, it is
> irrelevant whether the user chooses to rerun some Ant target later.
> 
> I am not really sure what your use case is, but I do not think this API is what
> you are looking for. Anyway I thought you were using #206196 to run profiler
> infrastructure?

The #206196 is used to simplify starting the profiled application. But we need to start the so-called profiler client as well. The client then attaches to the started application and displays the live results etc. I can not rely on the #206196 infra for starting the profiler client because it fails when re-runing profiling on ant based projects - the Rerun action implementation doesn't use startup extenders and directly executes the last used ant target with cached properties. 

Of course, I can solve this with a custom ant task but I would rather avoid it if possible.

(In reply to comment #23)
> I can not rely on the
> #206196 infra for starting the profiler client because it fails when re-runing
> profiling on ant based projects - the Rerun action implementation doesn't use
> startup extenders and directly executes the last used ant target with cached
> properties.

Earlier I suggested you file this as an issue blocking #206196. Either way, ActionProgress is not intended for this kind of use case - it is off topic here.

> I can solve this with a custom ant task

Which is how Debug Project is currently implemented: <nbjpdastart> generates a server port number to pass to the target JVM, and in parallel launches the IDE client. StartupExtender would probably be a cleaner solution, though.

Created attachment 116895 [details]
Minor updates to main patch

Planned for tomorrow.

Created attachment 116896 [details]
Updated patch for #64991

Created attachment 116897 [details]
Patch for #206208

core-main #f6c1c3eba99c

Integrated into 'main-golden', will be available in build *201203240400* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/f6c1c3eba99c
User: Jesse Glick <jglick@netbeans.org>
Log: #71515: API for blocking project actions.

Integrated into 'main-golden', will be available in build *201203250401* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/f6c1c3eba99c
User: Jesse Glick <jglick@netbeans.org>
Log: #71515: API for blocking project actions.