This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 180459 - Optional reset of InputOutput in extexecution
Summary: Optional reset of InputOutput in extexecution
Status: RESOLVED FIXED
Alias: None
Product: platform
Classification: Unclassified
Component: Execution (show other bugs)
Version: 6.x
Hardware: PC All
: P3 normal (vote)
Assignee: Petr Hejl
URL:
Keywords: API, API_REVIEW_FAST
Depends on:
Blocks:
 
Reported: 2010-02-09 01:52 UTC by soldatov
Modified: 2010-02-21 22:32 UTC (History)
5 users (show)

See Also:
Issue Type: ENHANCEMENT
Exception Reporter:


Attachments
the patch (11.63 KB, patch)
2010-02-10 06:47 UTC, Petr Hejl
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description soldatov 2010-02-09 01:52:09 UTC
Scenario:
- Create Arguments project
- Call context menu and select "Clean and Build" menu item
or
- Push "Run|Batch Build Main Project..." menu item, press "Select All" button and press "Clean and Build"
==> project is built correctly, but IDE cleans previous logs very quickly and user can't check build log

After "Clean and Build" in Batch mode I see such log ("Clean(Debug)", "Clean(Release)", "Build(Debug)" logs are removed):
dmake: defaulting to parallel mode.
See the man page dmake(1) for more information on setting up the .dmakerc file.
ifdefqe-02 --> 1 job
ifdefqe-02 --> 1 job
/opt/sun/sunstudio12.1/bin/dmake -f nbproject/Makefile-Release.mk SUBPROJECTS= .build-conf
/opt/sun/sunstudio12.1/bin/dmake  -f nbproject/Makefile-Release.mk dist/Release/SunStudio-Linux-x86/welcome_1
ifdefqe-02 --> 1 job
mkdir -p build/Release/SunStudio-Linux-x86
CC    -c -fast -g0 -o build/Release/SunStudio-Linux-x86/welcome.o welcome.cc
ifdefqe-02 --> Job errors
ifdefqe-02 --> 1 job
mkdir -p dist/Release/SunStudio-Linux-x86
CC    -o dist/Release/SunStudio-Linux-x86/welcome_1 build/Release/SunStudio-Linux-x86/welcome.o  
ifdefqe-02 --> Job errors
BUILD SUCCESSFUL (total time: 1s)
Comment 1 Alexander Simon 2010-02-09 03:28:06 UTC
Please, provide possibility to forbid reseting InputOutput:

ExecutionService.configureInputOutput(InputOutput inputOutput) {
        try {
            inputOutput.getOut().reset();
        } catch (IOException exc) {
            LOGGER.log(Level.INFO, null, exc);
        }
....
Comment 2 Alexander Simon 2010-02-09 03:30:22 UTC
This lack of functionality does not allow CND migrate to org.netbeans.api.extexecution API.
Comment 3 Jesse Glick 2010-02-09 11:24:04 UTC
I don't work on extexecution.
Comment 4 Petr Hejl 2010-02-09 12:58:14 UTC
It might be ease to do such enhancement technically. However I would like to know more details about your use case. Do you use your own InputOutput or one provided by default? Please be as abstract as possible (I do not know much about cnd).

Such configuration parameter could bias the API. As it could be potentially dangerous to provide an option to don't reset output for output windows managed by infrastructure which are reused by name...
Comment 5 Alexander Simon 2010-02-10 01:16:53 UTC
Some cnd actions consist from several steps (for example: clean and build).
Such actions implemented as several executable calls (for example: make clean and make build).
Old cnd executor use following logic:
- create or reuse existent InputOutput by IOProvider.getDefault().getIO(name, false), where name is project name+"(Clean, Build)"+host name. So cnd reuse same tab for each concrete project building.
- reset InputOutput
- execute make clean without reset InputOutput
- execute make build without reset InputOutput
As result tab content will consist from both logs (clean and build).

New execution API does not allow reuse InputOutput in second step without resetting.
It is a big regression from cnd point of view.

So my suggestion:
If InputOutput exists before ExecutionService.run() and ExecutionDescriptor has property doNotResetInputOutput the ExecutionService.run() does not reset InputOutput.

Alternative:
Change API of InputOutput:
- add method reset and remove reseting from OutputWriter
It allows user to create proxy of InputOutput with own resetting.
Comment 6 Alexander Simon 2010-02-10 01:18:48 UTC
From cnd point of view it is API defect.
Comment 7 Alexander Simon 2010-02-10 01:22:38 UTC
IMHO resetting of InputOutput is not responsibility of ExecutionService.
Comment 8 Vladimir Voskresensky 2010-02-10 02:12:54 UTC
to refine Alexander's suggestion:
- if client specify directly own IO => remember this as internal state resetInputOutput=false
- reset io based on resetInputOutput
Comment 9 Petr Hejl 2010-02-10 02:58:33 UTC
I'll try to avoid resetting with custom io (this will change API behavior and may broke clients who expect this). Would it be enough?

Definitely no a defect. Defect can't be something you've expected working different way (unless there is a clash in documented and real behavior).
Comment 10 Petr Hejl 2010-02-10 03:23:39 UTC
(In reply to comment #5)
> Alternative:
> Change API of InputOutput:
> - add method reset and remove reseting from OutputWriter
> It allows user to create proxy of InputOutput with own resetting.

This is IMHO doable even now as InputOutput is interface and OutputWriter is abstract class you can wrap the InputOutput and OutputWriter to delegete all methods to original ones while doing nothing on reset.
Comment 11 Alexander Simon 2010-02-10 04:45:09 UTC
>This is IMHO doable even now as InputOutput is interface and OutputWriter is
>abstract class you can wrap the InputOutput and OutputWriter to delegete all
>methods to original ones while doing nothing on reset.
Not doable due to implementation of OutputWriter. Only own implementation of OutputWriter without delegate is possible.
Comment 12 Alexander Simon 2010-02-10 04:49:43 UTC
>I'll try to avoid resetting with custom io (this will change API behavior and
>may broke clients who expect this). Would it be enough?
It is enough.

>Definitely no a defect. Defect can't be something you've expected working
>different way (unless there is a clash in documented and real behavior).
It is defect because client do not expect that run will clean ouutput tab.
Do you expect that command ls will clean terminal?
It is a same situation. Client prepared InputOutput and do not expect that run will clean tab.
Comment 13 Petr Hejl 2010-02-10 05:18:31 UTC
It is just your opinion based on personal expectations. Unless ls will state in doc "I won't ever clear terminal" it can. Maybe I wouldn't like it so I would file an enhancement to not to clear the terminal as I would think it is good. But I can't argue it is a defect.

Otherwise every enhancement in API would be qualified as defect in case there is >=1 persons who feel it should work different way.
Comment 14 Alexander Simon 2010-02-10 05:37:43 UTC
By the way it is defect/regression for cnd users (11% of all NB users).
Comment 15 Petr Hejl 2010-02-10 05:53:03 UTC
You replaced (or wrote some new, I don't know) feature with extexecution without validating it is doing what you want/expect. So now rather than asking for enhancement (or even better contributing patch+test) you are blaming the API? With statements like: "It is regression for our users". Not a fault of the API.

This discussion is pretty much useless, consuming my time while I'm trying to implement the enhancement.
Comment 16 Tomas Mysik 2010-02-10 06:30:04 UTC
(In reply to comment #12)
> It is defect because client do not expect that run will clean ouutput tab.

I disagree, exactly this behaviour would be a defect for us (PHP). If I rerun a task (script), I expect that I will see the output of exactly one task (like it is now and has always been).

> Do you expect that command ls will clean terminal?
> It is a same situation. Client prepared InputOutput and do not expect that run
> will clean tab.

IMHO it's not, ExtExecution API != terminal.
Comment 17 Petr Hejl 2010-02-10 06:46:59 UTC
Please review the change. It makes the reset optional (but enabled by default to keep the compatibility) for client managed IO. For infrastructure managed IO the behavior is not changed in any way.
Comment 18 Petr Hejl 2010-02-10 06:47:20 UTC
Created attachment 94038 [details]
the patch
Comment 19 Alexander Simon 2010-02-10 08:30:01 UTC
(In reply to comment #15)
> You replaced (or wrote some new, I don't know) feature with extexecution
> without validating it is doing what you want/expect. So now rather than asking
> for enhancement (or even better contributing patch+test) you are blaming the
> API? With statements like: "It is regression for our users". Not a fault of the
> API.
> 
> This discussion is pretty much useless, consuming my time while I'm trying to
> implement the enhancement.

Petr, sorry if I hurt you it wasn't my intention. And thanks for the quick implementation of ENH.
Comment 20 Petr Hejl 2010-02-16 02:38:35 UTC
I'll integrate the change tomorrow.
Comment 21 Petr Hejl 2010-02-17 06:23:03 UTC
Fixed in web-main #41d5a0f1bef2.
Comment 22 Quality Engineering 2010-02-17 22:00:54 UTC
Integrated into 'main-golden', will be available in build *201002180200* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/41d5a0f1bef2
User: phejl@netbeans.org
Log: #180459 Optional reset of InputOutput in extexecution
Comment 23 Alexander Simon 2010-02-18 05:41:40 UTC
Petr, thanks for the fix. It works!
Comment 24 Quality Engineering 2010-02-21 22:32:21 UTC
Integrated into 'main-golden', will be available in build *201002220200* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/682120e4665a
User: alexvsimon@netbeans.org
Log: fixed CND part of BZ#180459 Optional reset of InputOutput in extexecution