Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 342353 - Tycho-Surefire should use Toolchains
Summary: Tycho-Surefire should use Toolchains
Status: RESOLVED FIXED
Alias: None
Product: z_Archived
Classification: Eclipse Foundation
Component: Tycho (show other bugs)
Version: unspecified   Edit
Hardware: All All
: P3 enhancement with 2 votes (vote)
Target Milestone: ---   Edit
Assignee: Igor Fedorenko CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-04-09 09:41 EDT by Evgeny Mandrikov CLA
Modified: 2021-04-28 16:55 EDT (History)
3 users (show)

See Also:


Attachments
proposed fix (3.68 KB, patch)
2011-04-09 19:00 EDT, Evgeny Mandrikov CLA
no flags Details | Diff
Proposed fix (attempt 2) (11.14 KB, patch)
2011-06-29 05:23 EDT, Evgeny Mandrikov CLA
no flags Details | Diff
Proposed fix (attempt 3) (9.57 KB, patch)
2011-07-13 06:36 EDT, Evgeny Mandrikov CLA
no flags Details | Diff
Proposed fix (attempt 3) (11.01 KB, patch)
2011-07-14 05:11 EDT, Evgeny Mandrikov CLA
igor: iplog+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Evgeny Mandrikov CLA 2011-04-09 09:41:14 EDT
Maven-Surefire uses Toolchains to select Java, but Tycho-Surefire always uses same java as was used to launch Maven. In fact it relates to following issue - https://issues.sonatype.org/browse/TYCHO-490
Comment 1 Evgeny Mandrikov CLA 2011-04-09 11:01:05 EDT
What I want to achieve here is to be able to test JaCoCo with different JDKs - https://sourceforge.net/apps/trac/eclemma/wiki/MavenBuild#TestingwithdifferentJDKs
Patch available at https://github.com/sonatype/sonatype-tycho/pull/5
Comment 2 Evgeny Mandrikov CLA 2011-04-09 19:00:23 EDT
Created attachment 192890 [details]
proposed fix

Patch attached.
Comment 3 Tobias Oberlies CLA 2011-05-04 04:18:49 EDT
I'll look at the contribution once the Tycho sources are in parallel IP at Eclipse.
Comment 4 Tobias Oberlies CLA 2011-06-10 10:40:37 EDT
@Jan, Igor: Since you are way more familiar with the tycho-surefire-plugin, could you please review Evgeny's patch?

We are allowed to take changes from bugzilla, although we are not yet in parallel IP (see bug 342809#c46).
Comment 5 Tobias Oberlies CLA 2011-06-10 10:42:38 EDT
@Evgeny: It would certainly help if you could provide an (integration) test that verifies that your changes work as expected.
Comment 6 Evgeny Mandrikov CLA 2011-06-10 12:40:36 EDT
@Tobias: That's a definitely good idea to have a test on top of this, but at time of creation I didn't had enough free time, thus maybe patch itself can be improved. So, I'll try to take a look on this again and provide better patch with tests.

P.S. There is inconvenience for me : as was said by Igor - patches accepted only thru bugzilla. IMO would be better to have an ability to send also via GitHub.
Comment 7 Tobias Oberlies CLA 2011-06-10 12:48:01 EDT
(In reply to comment #6)
> P.S. There is inconvenience for me : as was said by Igor - patches accepted only
> thru bugzilla. IMO would be better to have an ability to send also via GitHub.

You should vote for bug 283749: Then we could take patches to Gerrit, which would be a huge improvement over the current process.
Comment 8 Evgeny Mandrikov CLA 2011-06-10 12:54:06 EDT
(In reply to comment #7)
> You should vote for bug 283749: Then we could take patches to Gerrit, which
> would be a huge improvement over the current process.

Indeed - would be cool! Thanks for the link - voted.
Comment 9 Evgeny Mandrikov CLA 2011-06-29 05:23:36 EDT
Created attachment 198808 [details]
Proposed fix (attempt 2)

New patch attached - with a better implementation and with integration test. Also was tested on a real project.
Comment 10 Evgeny Mandrikov CLA 2011-06-29 05:37:51 EDT
@Tobias, Jan, Igor: Can someone consider this patch?
Comment 11 Igor Fedorenko CLA 2011-07-12 16:23:32 EDT
This patch breaks a number of integration tests




[INFO] Command line:
	/bin/sh -c -jar /private/tmp/tycho-bootstrap.localrepo/org/eclipse/tycho/tycho-p2-runtime/0.13.0-SNAPSHOT/eclipse/plugins/org.eclipse.equinox.launcher_1.2.0.v20110502.jar -debug -consoleLog -nosplash -application org.eclipse.equinox.p2.publisher.FeaturesAndBundlesPublisher -source /workspaces/m2eclipse-igordev/sonatype-tycho/tycho-its/target/projects/TYCHO321deployableFeature/target/site -metadataRepository file:/workspaces/m2eclipse-igordev/sonatype-tycho/tycho-its/target/projects/TYCHO321deployableFeature/target/site/ -metadataRepositoryName feature -artifactRepository file:/workspaces/m2eclipse-igordev/sonatype-tycho/tycho-its/target/projects/TYCHO321deployableFeature/target/site/ -artifactRepositoryName 'feature Artifacts' -noDefaultIUs -compress
/bin/sh: -j: invalid option
Usage:	/bin/sh [GNU long option] [option] ...
	/bin/sh [GNU long option] [option] script-file ...
GNU long options:
	--debug
	--debugger
	--dump-po-strings
	--dump-strings
	--help
	--init-file
	--login
	--noediting
	--noprofile
	--norc
	--posix
	--protected
	--rcfile
	--restricted
	--verbose
	--version
	--wordexp
Shell options:
	-irsD or -c command or -O shopt_option		(invocation only)
	-abefhkmnptuvxBCHP or -o option
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 19.817s
[INFO] Finished at: Wed Jul 13 00:05:36 MSD 2011
[INFO] Final Memory: 56M/112M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.eclipse.tycho:tycho-p2-plugin:0.13.0-SNAPSHOT:feature-p2-metadata (default-feature-p2-metadata) on project feature: P2 publisher return code was 2 -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.eclipse.tycho:tycho-p2-plugin:0.13.0-SNAPSHOT:feature-p2-metadata (default-feature-p2-metadata) on project feature: P2 publisher return code was 2
	at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:199)
	at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:148)
	at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:140)
	at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:84)
	at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:59)
	at org.apache.maven.lifecycle.internal.LifecycleStarter.singleThreadedBuild(LifecycleStarter.java:183)
	at org.apache.maven.lifecycle.internal.LifecycleStarter.execute(LifecycleStarter.java:161)
	at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:314)
	at org.apache.maven.DefaultMaven.execute(DefaultMaven.java:151)
	at org.apache.maven.cli.MavenCli.execute(MavenCli.java:445)
	at org.apache.maven.cli.MavenCli.doMain(MavenCli.java:168)
	at org.apache.maven.cli.MavenCli.main(MavenCli.java:132)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
	at java.lang.reflect.Method.invoke(Method.java:597)
	at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced(Launcher.java:290)
	at org.codehaus.plexus.classworlds.launcher.Launcher.launch(Launcher.java:230)
	at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode(Launcher.java:409)
	at org.codehaus.plexus.classworlds.launcher.Launcher.main(Launcher.java:352)
Caused by: org.apache.maven.plugin.MojoFailureException: P2 publisher return code was 2
	at org.eclipse.tycho.plugins.p2.AbstractP2MetadataMojo.generateMetadata(AbstractP2MetadataMojo.java:114)
	at org.eclipse.tycho.plugins.p2.AbstractP2MetadataMojo.execute(AbstractP2MetadataMojo.java:88)
	at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:107)
	at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:195)
	... 19 more
Comment 12 Evgeny Mandrikov CLA 2011-07-12 16:47:55 EDT
Hi Igor,
Thanks for the testing.
DefaultEquinoxLauncher uses path to jvm executable from EquinoxLaunchConfiguration, so this path wasn't configured.
I can imagine two ways to fix this :
# handle this case in DefaultEquinoxLauncher - some kind of backward compatibility
# find place where EquinoxLaunchConfiguration was constructed and configure correct path
Could you please give me an advice which way do you prefer, if you want me to modify patch? Or maybe you prefer to fix it by yourself?
Comment 13 Igor Fedorenko CLA 2011-07-13 00:21:29 EDT
I'd handle this in DefaultEquinoxLauncher, i.e. move back the logic to calculate jvm path if it is not provided via configuration.
Comment 14 Evgeny Mandrikov CLA 2011-07-13 06:36:38 EDT
Created attachment 199563 [details]
Proposed fix (attempt 3)

New patch attached – with restored behaviour of DefaultEquinoxLauncher when JVM executable was not specified in LaunchConfiguration
Comment 15 Igor Fedorenko CLA 2011-07-14 04:26:35 EDT
For proper attribution purposes we require patches in git-format-patch format.
Comment 16 Evgeny Mandrikov CLA 2011-07-14 05:11:36 EDT
Created attachment 199644 [details]
Proposed fix (attempt 3)

Patch in git-format-patch format.
Comment 17 Igor Fedorenko CLA 2011-07-14 10:15:23 EDT
Applied the patch. Thank you.
Comment 18 Tobias Oberlies CLA 2011-07-14 10:52:10 EDT
Is this new feature self-explaining, or should it be documented in the Tycho wiki [1]? Is this worth advertising as new feature in the release notes [2]?

[1] http://wiki.eclipse.org/Tycho
[2] http://wiki.eclipse.org/Tycho/Release_Notes/0.13
Comment 19 Evgeny Mandrikov CLA 2011-07-14 11:29:49 EDT
@Tobias, I think that this feature is valuable enough and deserves to be mentioned in wiki and in release notes, however I suppose that final decision should not be made by me (and maybe this question wasn't for me?) ;)
Comment 20 Tobias Oberlies CLA 2011-07-15 07:35:16 EDT
I don't know barely anything about toolchains, so I trust your judgement.

Could you document the feature in the wiki (on a new or existing page) and link to that documentation on the 0.13. release notes page?