Skip to content
This repository has been archived by the owner on Apr 21, 2023. It is now read-only.

Switch to Maven/Tycho (EXPERIMENT) #2052

Closed
LorenzoBettini opened this issue Jan 13, 2023 · 46 comments
Closed

Switch to Maven/Tycho (EXPERIMENT) #2052

LorenzoBettini opened this issue Jan 13, 2023 · 46 comments
Labels
Milestone

Comments

@LorenzoBettini
Copy link
Contributor

As said here eclipse/xtext#2133 I'd like to try to switch to Maven/Tycho

@LorenzoBettini
Copy link
Contributor Author

I started working on that in my fork, this branch https://github.com/LorenzoBettini/xtext-core/tree/lb_2052_maven_tycho

For the moment org.eclipse.xtext.util and org.eclipse.xtext are compiling!

I also set up a GitHub Actions workflow since I'll deal with Jenkins later.
In case, later, we can remove the GitHub Actions workflow.

@LorenzoBettini
Copy link
Contributor Author

@LorenzoBettini
Copy link
Contributor Author

@cdietrich in xtext.ide the MANIFEST wants org.eclipse.lsp4j with at least 0.19. I can't find a p2 repo providing that. Do you know where I can find it?

@cdietrich
Copy link
Member

Please check Xtext.setup it contains the url

@LorenzoBettini
Copy link
Contributor Author

Right! Thanks! I'm currently not using Xtext.setup and haven't thought about that.

@LorenzoBettini
Copy link
Contributor Author

org.eclipse.xtext.ide also builds!

@LorenzoBettini
Copy link
Contributor Author

org.eclipse.xtext.testing builds and tests run!

@LorenzoBettini
Copy link
Contributor Author

@cdietrich should the Xtend files be processed during the build? If so, I guess I should use the latest released version of the xtend-maven-plugin, because it's not in this repo, right

@LorenzoBettini
Copy link
Contributor Author

org.eclipse.xtext.xtext.generator and wizard build!

@cdietrich
Copy link
Member

regarding xtend: yes we do the same with the gradle build.
in xtext-eclipse and xtext-xtend we had a https://github.com/eclipse/xtext-xtend/blob/8521139f5652be55444114de850af23aee37ab47/releng/org.eclipse.xtend.tycho.parent/pom.xml#L13 property

@LorenzoBettini
Copy link
Contributor Author

xtext.ide.tests run and pass in the Maven build! :)

@LorenzoBettini
Copy link
Contributor Author

Build and tests are performed also on macOS and Windows

@LorenzoBettini
Copy link
Contributor Author

In the current maven build script I see

-Dmaven.repo.local=.m2/repository

I guess it's useless since the Jenkins workspace is ephemeral anyway, so I removed that.

It's installing in

/home/jenkins/.m2/repository/org/eclipse/xtext/

but it should not harm anything else...

do you want to me to put it back anyway?

@cdietrich
Copy link
Member

i assue its there to be on the safe side, also locally

@LorenzoBettini
Copy link
Contributor Author

?

@cdietrich
Copy link
Member

i assume its there to be on safe side also in local builds

@LorenzoBettini
Copy link
Contributor Author

OK, I restored that.

We have a green build on Jenkins as well :) (just Maven/Tycho, NO gradle involved at all)

https://ci.eclipse.org/xtext/job/xtext-core/job/lb_2052_maven_tycho/

And the archived artifacts are there as well:

https://ci.eclipse.org/xtext/job/xtext-core/job/lb_2052_maven_tycho/lastSuccessfulBuild/artifact/

If I'm not wrong, with the same layout.

@cdietrich
Copy link
Member

looks like there are still 7 tests missing

@LorenzoBettini
Copy link
Contributor Author

OK, I'll create the WIP PR, detailing first the (temporary) manual steps to use the project from Eclipse and then, hopefully later today, I'll leave a few notes in the PR on the relevant changes and things that I think are still left to do

@LorenzoBettini
Copy link
Contributor Author

looks like there are still 7 tests missing

I'll write a few additional details about that in the PR.
However, I detected a few duplicates (at least, comparing the run tests in Eclipse), which are correctly discarded during the Maven build.

@cdietrich
Copy link
Member

for missing tests. might be a naming convention problem e.g. LexerSLComment

@LorenzoBettini
Copy link
Contributor Author

for missing tests. might be a naming convention problem e.g. LexerSLComment

For sure! I thought I caught them all, but that one escaped me!

@LorenzoBettini
Copy link
Contributor Author

caught also other two classes that I've just renamed

@cdietrich
Copy link
Member

i also see diffs in
org.eclipse.xtext.parser.unorderedGroups
org.eclipse.xtext.testing.tests
org.eclipse.xtext.testing.tests.junit5
org.eclipse.xtext.validation.junit5
but did not look into that yet
a.txt
b.txt

@LorenzoBettini
Copy link
Contributor Author

This one

package org.eclipse.xtext.testing.tests.junit5;

import org.junit.platform.runner.JUnitPlatform;
import org.junit.platform.suite.api.SelectPackages;
import org.junit.platform.suite.api.UseTechnicalNames;
import org.junit.runner.RunWith;

/**
 * Test suite that will pick up all Junit5 tests from the listed package and run them
 * when the suite is executed with Junit4.
 */
@RunWith(JUnitPlatform.class)
@UseTechnicalNames
@SelectPackages("org.eclipse.xtext.testing.tests.junit5")
@SuppressWarnings("deprecation")
public class RunJUnit5Suite {

}

is currently not executed in Eclipse either; in particular, I don't understand why we should want to do that.

For example,

@ExtendWith(InjectionExtension.class)
@InjectWith(InjectionExtensionNested3Test.MyInjectorProvider.class)
public class InjectionExtensionNested3Test {

why do we want to do that?

@LorenzoBettini
Copy link
Contributor Author

Now it's 4735 instead of 4738.

@cdietrich
Copy link
Member

in gradle this executes the junit5 tests from junit4
in eclipse it works when you select junit4 in the launch config dialog

@cdietrich
Copy link
Member

but then the should be run?

@LorenzoBettini
Copy link
Contributor Author

yes, I spotted now that one as well, looks like

The Surefire Plugin excludes nested classes by default: http://maven.apache.org/surefire/maven-surefire-plugin/test-mojo.html#excludes ; By removing all default exclusions the nested classes get executed too, JUnit 4 and JUnit 5.

I'm trying that now...

@cdietrich
Copy link
Member

what is with FileAwareTestLanguageValidationJunit5Test?

@LorenzoBettini
Copy link
Contributor Author

yes, I spotted now that one as well, looks like

The Surefire Plugin excludes nested classes by default: http://maven.apache.org/surefire/maven-surefire-plugin/test-mojo.html#excludes ; By removing all default exclusions the nested classes get executed too, JUnit 4 and JUnit 5.

I'm trying that now...

Yes, that was the culprit: all tests in .xtext.testing are now executed just like in Eclipse.

@LorenzoBettini
Copy link
Contributor Author

what is with FileAwareTestLanguageValidationJunit5Test?

I guess that's the one that remains to be investigated.

@LorenzoBettini
Copy link
Contributor Author

Now it's 4736 instead of 4738.

The two tests of FileAwareTestLanguageValidationJunit5Test are missing.

In the Maven output, I see that JUnit 4 is used (while in .xtext.testing JUnit 5 was used).

I'll have to investigate that (maybe tomorrow or this evening)

@LorenzoBettini
Copy link
Contributor Author

Done! 4738 as before! :)

It took me the whole evening to solve that, so further notes on the PR will have to wait til tomorrow.

@SimonCockx
Copy link

SimonCockx commented Jan 17, 2023

@LorenzoBettini Just out of curiosity: why are you switching to Maven/Tycho?

We've been working with Maven/Tycho for a couple of years now, and we are actually planning to switch to Gradle at this very moment. We've had too many problems with Tycho in the past (e.g., corrupting the .m2/repository folder when switching versions, having problems with transitive dependencies when a pure Maven project relies on our Tycho project, etc etc)

@cdietrich
Copy link
Member

cdietrich commented Jan 17, 2023

there are multiple problems with gradle

@SimonCockx
Copy link

@cdietrich thanks for the update, that's interesting. Luckily for me, all of these problems don't apply, so that's a reassurance. :)

This still means that building Xtext projects with Gradle is supported, right?

@LorenzoBettini
Copy link
Contributor Author

@SimonCockx IMHO (and experience) Maven has the main feature of being ONLY a build tool, based on a simple concept of lifecycle (which can be shown in a single presentation slide) that is as rigid as possible, forcing you to do things in a single way, without any flexibility... but it also has drawbacks... ;) jokes aside, I think that's how a build tool must be. Not to mention that if you know Maven and look at any POM out there you'll immediately understand what's going on (and you know where to look at). From what I know of Gradle, you definitely cannot say that.

If you also deploy Maven artifacts, you have to disable Tycho pom preprocessing, otherwise you have the problems you mention.

The idea is that you can't do strange and complex things because if you try to fight Maven you'll lose. That's another great positive feature ;) Finally, if you deploy Maven projects that are meant to be also bundles and Eclipse plugins (and so you also publish p2 sites), stick with Maven/Tycho, because that's native to these mechanisms. Gradle flexibility is just a drawback, not a positive thing. Fight me ;)

@cdietrich
Copy link
Member

well at least when nobody shows mercy and fixes xtext/xtext-gradle-plugin#207 and xtext/xtext-gradle-plugin#203 only if you wont use any xtend at all in your codebase

@SimonCockx
Copy link

SimonCockx commented Jan 17, 2023

@LorenzoBettini well, if we ignore performance for a moment, for one the documentation on Gradle is fantastic, which I really can't say about Tycho. ;)

Seriously though, I know there are solutions to the problems I've mentioned, but there are really no good guides explaining them, and our team lacks a person with good knowledge about it. Which is another important reason for our plan to switch to Gradle.

I get your point on having too much flexibility, but as someone relatively new to Java build systems, I must say that a graph-based representation of tasks comes much more naturally to me than a (seemingly arbitrary, at least for me :) ) rigid lifecycle line. With the additional benefit of parallelization; another performance boost and less waste of precious development time!

That being said, I lack experience of working with Gradle in a big project, so I'll stop fighting. Maybe in a couple of years I'll get back in the ring ;)

@minesh-s-patel
Copy link

@LorenzoBettini I totally agree with your mvn vs gradle argument. I have seen massively over engineered build systems built with gradle ('with great power comes the ability to screw things up'). That being said - using maven tycho has been a PITA for @SimonCockx and our team and we want to move off it. We thought that gradle was the strategic way forward but it sounds like we just need to convert out build to be pure (no tycho) mvn.

@cdietrich
Copy link
Member

with repo merge this is done

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants