eclipse / xtext-eclipse

xtext-eclipse
Eclipse Public License 2.0
49 stars 73 forks source link

Move to Maven/Tycho for projects and build infrastructure #1992

Closed LorenzoBettini closed 1 year ago

LorenzoBettini commented 1 year ago

See also https://github.com/eclipse/xtext-lib/issues/499 and https://github.com/eclipse/xtext-core/issues/2052 https://github.com/eclipse/xtext-extras/issues/858 https://github.com/eclipse/xtext-xtend/issues/1445

LorenzoBettini commented 1 year ago

@cdietrich @szarnekow I'm "harmonizing" also xtext-eclipse, which was already based on Tycho.

I'm getting failures in this test org.eclipse.xtext.builder.tests.ParallelBuildEnabledTest:

public class ParallelBuildEnabledTest {
    @Test
    public void testIsParallelBuildEnabledInWorkspace() {
        int maxConcurrentProjectBuilds = new ScopedPreferenceStore(InstanceScope.INSTANCE, ResourcesPlugin.PI_RESOURCES).getInt("maxConcurrentBuilds");
        assertEquals("parallel build was not enabled", 8, maxConcurrentProjectBuilds);
    }

    @Test
    public void testBuilderSchedulingRuleEnabledInWorkspace() {
        String schedulingRule = new ScopedPreferenceStore(InstanceScope.INSTANCE, Activator.PLUGIN_ID).getString(XtextBuilder.BuilderPreferences.PREF_SCHEDULING_RULE);
        assertEquals("non-default scheduling rule was not enabled", SchedulingOption.ALL_XTEXT_PROJECTS.name(), schedulingRule);
    }

}

The first one fails with

java.lang.AssertionError: parallel build was not enabled expected:<8> but was:<1>

The second one with:

org.junit.ComparisonFailure: non-default scheduling rule was not enabled expected:<[ALL_XTEXT_PROJECTS]> but was:<[]>

They fail the same way both from Eclipse and from the build.

I don't think I touched anything special to make them fail. Do you have any clue of what might be the cause of failure?

cdietrich commented 1 year ago

releng/org.eclipse.xtext.tycho.tests.parent/pluginCustomization.ini is it picked?

LorenzoBettini commented 1 year ago

Right! that was the curlprit: I changed the directory of tycho parent tests! I updated the path and it works. Thanks!

LorenzoBettini commented 1 year ago

There's another small problem I'm facing while harmonizing the build. Looks like in this project the dev-bom is kind of disturbing.

In the parent POM I'm importing it (in the master version xtext-eclipse never uses the BOM):

    <dependencyManagement>
        <dependencies>
            <dependency>
                <groupId>org.eclipse.xtext</groupId>
                <artifactId>xtext-dev-bom</artifactId>
                <!--
                TODO: update it
                The dev BOM should be in the same Git repository of this one
                -->
                <version>${xtext-current-release-version}</version>
                <type>pom</type>
                <scope>import</scope>
            </dependency>
        </dependencies>
    </dependencyManagement>

where

<xtext-current-release-version>2.30.0.M1</xtext-current-release-version>

when building I'm activating the profile useJenkinsSnapshots that redefines that property like that

<xtext-current-release-version>${project.version}</xtext-current-release-version>

This does not work for the xtext.logging fragment:

<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
    <modelVersion>4.0.0</modelVersion>
    <parent>
        <groupId>org.eclipse.xtext</groupId>
        <artifactId>xtext-eclipse</artifactId>
        <version>2.30.0-SNAPSHOT</version>
    </parent>
    <packaging>eclipse-plugin</packaging>
    <artifactId>org.eclipse.xtext.logging</artifactId>
    <version>1.2.24-SNAPSHOT</version>

...
</project>

because it would try to download the dev-bom with version 1.2.24-SNAPSHOT.

It's not a big issue, because in xtext-eclipse the bom was not used before, and in this repository I can remove it.

But just to leave a note/reminder here that when merging everything, we'll have to have a different parent ONLY for artifacts that must be released to central and put the bom import only in that parent.

cdietrich commented 1 year ago

the dev bom should be project.version. why cant this be downloaded? am also not sure if we can have a different version in pom and manifest

cdietrich commented 1 year ago

the xtext-current-release-version var name highly disturbs me. is it bootstrrap version for xtend or something else?

cdietrich commented 1 year ago

i guess you can redefine the variable and then we fix the adapt versions script

LorenzoBettini commented 1 year ago

see the contents of org.eclipse.xtext.logging above. In that only project's POM, project.version evaluates to 1.2.24-SNAPSHOT. Remember that in Maven when you inherit from the parent POM the properties relative to the project are evaluated in each single project.

In all git repositories but xtext-lib, the version of the bom is project.version only when using the latest snapshots from jenkins, otherwise, it must be the latest released version.

The xtext-current-release-version is a temporary property to refer to the currently latest released version; this is overridden in the profile using the latest snapshots.

What I'm saying is that the bom cannot be used like that in the presence of xtext.logging. As I said, in your current master, xtext-eclipse does not use the bom.

cdietrich commented 1 year ago

In all git repositories but xtext-lib, the version of the bom is project.version only when using the latest snapshots from jenkins, otherwise, it must be the latest released version.

no we need to use current-version-SNAPSHOT everywhere for the bom. otherwise we will never be able to change the bom ever again

cdietrich commented 1 year ago

googling also give hints on ${revision} var. what is that one good for? i assume for passing from outside

LorenzoBettini commented 1 year ago

As I said, it's a temporary workaround for me that allows me to test locally against latest released versions of Maven artifacts. But don't worry, in the CI (JIRO) we always use latest snapshots by enabling the profile (as it was done before):

        <!--
        This is the version of the currently released latest version that we
        use during the Maven builds for artifacts that are not in this Git repository
        (and thus, not in this Maven reactor).
        It is redefined in the profile "useJenkinsSnapshots" to take the same version
        of this project.
        -->
        <xtext-current-release-version>2.30.0.M1</xtext-current-release-version>

        <!--
        This is the version of the currently released latest version of the xtend-maven-plugin that we
        use during the Maven builds for processing Xtend files.
        -->
        <xtend-maven-plugin-version>2.30.0.M1</xtend-maven-plugin-version>

...

    <profiles>
        <profile>
            <id>useJenkinsSnapshots</id>
            <properties>
                <!-- redefines the property for versions of artifacts not in this Git repository,
                    so that it takes the same snapshot version of this project. -->
                <xtext-current-release-version>${project.version}</xtext-current-release-version>
            </properties>
            <repositories>
                <repository>

Don't worry, it was more a note to myself for the merging. Currently, in xtext-eclipse the bom is not imported otherwise you would the same failure I'm seeing because to harmonize all the parents I'm copying them. For xtext-eclipse I cannot import the bom, which, anyway, is needed only for pure Maven artifacts.

probably revision would not help.

cdietrich commented 1 year ago

but then this problem will be there tooo?

but it also means we are testing bad. i would have expected something like https://github.com/eclipse/xtext-eclipse/pull/1932/files and https://github.com/eclipse/xtext-xtend/pull/1409/files

LorenzoBettini commented 1 year ago

No: it's just a matter of having a parent POM that is used for bundle projects (not importing the BOM) and another parent POM, which inherits from the first one, used for Maven projects: this latter imports the BOM.

cdietrich commented 1 year ago

see my update of privoous comment. i would expect to find a upstream repo list

LorenzoBettini commented 1 year ago

Sorry, I'm not following. Maybe your answers are too interleaved my answers. I'm not sure what your concerns are.

cdietrich commented 1 year ago

we need to make use of the snapshot dev bom. and also consume the snapshot version of upstream artifacts => this is why we e.g. do this one https://github.com/eclipse/xtext-maven/blob/d0510dbe8088019f9a4ce7c71e6b7920bd620b5a/org.eclipse.xtext.maven.parent/pom.xml#L287 i thought you do this in all repos

LorenzoBettini commented 1 year ago

I still haven't worked on xtext-maven.

In xtext-eclipse you're not using the bom right now (it's also useless because they are all bundles processed entirely by Tycho).

cdietrich commented 1 year ago

yes and no. we use it in gradle https://github.com/eclipse/xtext-extras/blob/8788ec44652a1935101e3656cad5d25e4fc2a8ad/gradle/upstream-repositories.gradle#L19 https://github.com/eclipse/xtext-extras/blob/8788ec44652a1935101e3656cad5d25e4fc2a8ad/build.gradle#L32

and in eclipse we build with old tycho that does not do the "look at maven crap"

LorenzoBettini commented 1 year ago

If you want the dev bom in xtext-eclipse now (I repeat, it is not being used in master): the only fast solution I can think of is to override the dev bom import in xtext.logging. By the way, why doesn't that project use the same version as the other ones?

cdietrich commented 1 year ago

i thing we need some moderation here. cc @szarnekow i still dont see a picture how we create a milestone testing the whoile thing.

i read your proposal is to do a repo merge and then do a milestone i would have create a milestone based on your branch with splitted repo.

to override the dev bom import in xtext.logging. By the way, why doesn't that project use the same version as the other ones?

the version of fragment is the version of the host. i cannot tell you why

LorenzoBettini commented 1 year ago

Actually, what I'm working on is try to create a p2 repository with signed contents with the SPLITTED repo, so that we can try to do a fake/milestone/whatever release with the SPLITTED repo. Isn't that what you asked for?

cdietrich commented 1 year ago

Isn't that what you asked for?

yes. but therefore i want stuff to be pulled from upstream snapshots/repos in jenkins and not the previously built milestone. in milestone buidls tycho 2.7.5 pulls stuff from maven repos and will den mimi cause it cannot find it. dont know what pure maven will do and tycho will behave differently for snapshots and milestones so will will see it explode only when it explodes

LorenzoBettini commented 1 year ago

from my findings and understanding, tycho explodes due to the way the Xtext p2 repo is currently being built right now with the strange reference to local-repo (or something), that is, due to the not completely correct metadata. This should go away once the p2 repo is built correctly.

Anyway, I'll find a way to circumvent the problem of xtext.logging and re-introduce the bom also in xtext-eclipse.

cdietrich commented 1 year ago

naive question: project.parent.version what maven versions does this work in and what not?

LorenzoBettini commented 1 year ago

Actually, I'm checking that property right now :)

LorenzoBettini commented 1 year ago

Looks like it's working: it's just a matter of redefining that property in xtext.loggin and we can import the BOM also in xtext-eclipse: https://github.com/LorenzoBettini/xtext-eclipse/commit/f035c3a2f7ca137b79a2f22af867f62ed34a1ca3#diff-ff2da789bfe064325263bcdc949bf700f7435c2337026426311ccc462a67b8ffR13

cdietrich commented 1 year ago

with repo merge this is done