Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SuppressErrors #49

Merged
merged 2 commits into from
Feb 2, 2018
Merged

Conversation

ralfgrossklaus
Copy link
Contributor

Added suppressErrors argument to EclipseApp in order to prevent message dialogs on PDE errors.

Added suppressErrors argument to EclipseApp in order to prevent message dialogs on PDE errors.
@ralfgrossklaus
Copy link
Contributor Author

Hi! Here is the fix for preventing the message boxes on errors while running EclipseApps (e.g. PDE). Sorry that it took a little longer than expected

Regards, Ralf

.filter(Objects::nonNull)
.filter(arg -> !arg.startsWith("--launcher"))
.collect(toList());
this.args = ImmutableList.copyOf(filteredArgs);
return this;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this filter for? If I pass --launcher.someFlag, it will be silently discarded, which will be quite hard to debug.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I needed that for the P2BootstrapInstallationTest to pass, since unknown arguments will lead to an error (See log output at the end of this post). As i understood it, EquinoxLauncher is an alternate way to startup equinox, without having to use the native Eclipse launchers? The "--launcher" arguments are usually meant for the eclipse.exe or eclipsec.exe launchers and not passed further to Equinox. Please correct me, if my understanding is wrong. I would be glad for any better solution.

Installation failed.
!SESSION 2018-02-02 20:24:20.472 -----------------------------------------------
eclipse.buildId=unknown
java.version=1.8.0_112
java.vendor=Oracle Corporation
BootLoader constants: OS=macosx, ARCH=x86_64, WS=cocoa, NL=de_DE
Framework arguments: --launcher.suppressErrors -application org.eclipse.equinox.p2.director -repository http://download.eclipse.org/eclipse/updates/4.5/R-4.5.2-201602121500/ -installIU org.eclipse.core.runtime -profile profile -destination file:///var/folders/4d/z5fm5n5x5gsdzwswfrfd7b100000gn/T/junit191409443383839977/installed
Command-line arguments: --launcher.suppressErrors -application org.eclipse.equinox.p2.director -clean -consolelog -repository http://download.eclipse.org/eclipse/updates/4.5/R-4.5.2-201602121500/ -installIU org.eclipse.core.runtime -profile profile -destination file:///var/folders/4d/z5fm5n5x5gsdzwswfrfd7b100000gn/T/junit191409443383839977/installed

!ENTRY org.eclipse.equinox.p2.core 4 0 2018-02-02 20:24:20.474
!MESSAGE Unknown option --launcher.suppressErrors. Use -help for a list of known options.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhhh, I see. You are correct about EqunoxLauncher.

I think a safer bet here would be to filter --launcher.suppressErrors only. There are quite a few --launcher flags, and it may be that some of them are supported by EquinoxLauncher.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allright. I changed that

replaced the argument filter for EquinoxLauncher for more precise filtering
@nedtwigg nedtwigg merged commit 8844ff1 into diffplug:master Feb 2, 2018
hacki11 pushed a commit to hacki11/goomph that referenced this pull request Feb 4, 2018
@hacki11 hacki11 mentioned this pull request Feb 4, 2018
nedtwigg pushed a commit that referenced this pull request Feb 5, 2018
* Allow providing a pde bootstrap installation like p2bootstrap feature does with a pdebootstrapurl override
This can be an advantage if you are behind restrictive proxies bc by default pde is downloaded from p2 repos

* change whitespaces to tabs for spotless

* add #49 to changes.md

* mixed up pull requests
@hacki11
Copy link
Contributor

hacki11 commented Feb 17, 2018

the workspace and log is cleared immediately after eclipse exits. with the error dialog i was able to read the log until a manual click. how do i preserve the log in case of error with suppressErrors activated?

edit: this could be a possible solution #55, what do you think?

@ralfgrossklaus
Copy link
Contributor Author

Hi! Sorry for the late response. Is there already a solution for the cleared workspace? Otherwise I would fix this, as i have another workspace issue that i am going to fix: Since we are building more than one product, there might be parallel (multithreaded) executions of PDE which is currently not possible, since the PdeInstallation always uses the same workspace directory. My idea is to make this configurable. Any suggestions?

Regards, Ralf

@hacki11
Copy link
Contributor

hacki11 commented Feb 27, 2018

with this feature #55 one can activate consoleLog and get all the errors during the gradle run. no need for log files or workspace anymore.

for a different workspace dir you could extend PdeInstallation and use PdeBuildTask to provide custom workspace directory. Another option would be to use PdeProductBuildConfig::id as a unique part of the workspace directory. but i think this deserves a separate issue.

@nedtwigg
Copy link
Member

A few other tips:

  • The cleared workspace problem can be fixed by removing this line. Looks like I was overzealous making it the default, rather than an opt-in. I thought it was a good default, but if it's undebuggable by default then I might have been wrong. Perhaps we ought to instead combine (-consoleLog, -debug, and -launcher.suppressErrors) into an opt-in headlessVerbose() or something like that.
  • Here is where we set the workspace for PdeInstallation.
  • For controlling the workspace, I would recommend updating the existing WorkspaceRegistry.
    • One option is to create random folders, but I don't think that's a good idea because randomness makes testing harder.
    • Content-addressing is a better option. For creating workspace IDE's, we take a hash of the absolute path to the workspace project, and concat it with a the project name for easy-to-read debugging. For your use-case, maybe it should be workspaceDir(Project project, Task task) and the hash can be a combination of the project's rootDir absolute path and the task's path.

@ralfgrossklaus ralfgrossklaus deleted the Ignore_Errors_Switch branch February 28, 2018 22:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants