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

Cannot use a plain version number when declaring a tile #63

Open
rvowles opened this issue May 17, 2016 · 12 comments
Open

Cannot use a plain version number when declaring a tile #63

rvowles opened this issue May 17, 2016 · 12 comments

Comments

@rvowles
Copy link
Member

rvowles commented May 17, 2016

using:

                <tiles>
                    <tile>nz.co.xxxxxx:xxxxx-tile-common:1.1-SNAPSHOT</tile>
                </tiles>

fails as it is trying to get the xml file. You have to wrap it in a fixed version range.

@talios
Copy link
Member

talios commented May 17, 2016

Interesting - is that a deployed SNAPSHOT (timestamped) or a local SNAPSHOT ( -SNAPSHOT file? ).

If the former, I can almost understand it, since you need to resolve the timestamp of the latest version from the metadata, does it work AFTER using a range ( i.e. there is metadata locally ).

@talios
Copy link
Member

talios commented Jun 23, 2016

Actually this is weird, I was doing this just last night. However, my SNAPSHOT was in my local repository - which you don't mention if it was or not? I don't recall off hand how we're resolving things, but in my recent usage of Aether directly getting SNAPSHOTs also seemed fiddly.

@bvella
Copy link
Contributor

bvella commented Dec 30, 2018

I have numerous places where I have SNAPSHOT tiles declared and they seem to work just fine. I have places where i use a tile in the same reactor build and other places where I use the tile and expect it to be in the local repo. This is an example of a project structure that I have:

  • commons
    • jooq
      • generator-tile
      • generator-test (dependency on generator-tile in same reactor build)
  • app (dependency on generator tile in different build)

Lets say commons 1.0 is released and the current development version is 1.1-SNAPSHOT. When I build commons, commons-jooq-generator-test will pick up the snapshot version of the tile defined as <tile>commons:commons-jooq-generator-tile:${project.version}</tile> within the same reactor build. Also, if I only build commons-jooq-generator-test, it will pick the last built tile from the local repo, or fail if this version was never built locally (similar to what happens for any other unavailable dependency).

If I do some changes to commons-jooq in version 1.1-SNAPSHOT that I want to use in app before commons 1.1 is released, I start depending on that version from app. When app is built, the tile is retrieved from the local repo as well, or fail if this version has never been built locally.

As such, either replication steps are provided or this issue can probably be closed

@talios
Copy link
Member

talios commented Jan 3, 2019 via email

@bvella
Copy link
Contributor

bvella commented Jan 4, 2019

Do you mean here that 1.1-SNAPSHOT is pulled from the repo and not from the sources when only building the app?

Yes I mean that the snapshot is pulled from the local repo, if the snapshot was already built locally, otherwise it will fail (we do not deploy SNAPSHOTs).

What i think would happen if each tile is included as a dependency is that -am will add the tile modules to the project list to be built if these tiles make part of the same tree, otherwise, it should work as it is today.

As an experiment, I added a bit of code to add the tile dependencies to the project models while injecting the tiles. However, although the dependencies show up in the effective pom, it seems that maven does not consider the modified model when building the project graph to build. Running mvn -pl :project-using-tile -am did not pick up the tile, event though the tile is part of the project and running mvn picks up the tile in the project list

@bvella
Copy link
Contributor

bvella commented Jan 4, 2019

After doing some debugging, it seems that the project list is evaluated before it is handed off to the lifecycle, so even though dependencies can be added to the model, they are not added in time to be considered by maven's -am/-amd switches

@bvella
Copy link
Contributor

bvella commented Jan 4, 2019

Actually, I did some more digging and I think there is a way to do this in the following way:

Maven first read the list of projects, then sorts them based on the dependency graph, then filters the list of projects (-pl, -am, -amd, etc) then finally hands this list to the lifecycles. One can register an extension that can inject components that replace maven's default components, one such component being DefaultProjectBuilder, which reads and builds the project model pojo.

If projects using tiles add .mvn/extensions.xml file at the root of the project, then the tiles extension is available before the projects are read:

<?xml version="1.0" encoding="UTF-8"?>
<extensions>
    <extension>
        <groupId>repaint.io</groupId>
        <artifactId>maven-tiles</artifactId>
        <version>[version]</version>
    </extension>
</extensions>

The tiles extension can then inject a subclass of DefaultProjectBuilder that adds the tile dependencies to the project from the tiles config. Maven should then sort the projects and filter accordingly, after which, the filtered projects are handed to tiles extension for injecting the actual tile.

For reference, i used https://maven.apache.org/docs/3.3.1/release-notes.html#Core_Extensions and https://github.com/Code-House/maven-osgi-resolver/tree/3.3.9-a (and some 30 minutes of debugging in maven code to understand how the project graph is built)

I will see if I find a couple of hours to try this out and report back

@bvella
Copy link
Contributor

bvella commented Jan 9, 2019

I found some time and tried this. Turns out it is possible in the manner described in my previous comment. works as it currently does without the extensions.xml file but when this is added, mvn -pl :project -am correctly picks up the tile as a dependency orders it before the project. Seems to be working great, and as a happy consequence, I restored compatibility with maven 3.3.9. Added to #84.

I am finding it hard to write an integration test though, and a quick explanation of how the existing ones run might help. I did all testing manually on my projects.

@talios
Copy link
Member

talios commented Jan 15, 2019

I think I'd rather this as a separate/isolated PR's for the various changes here, and some squashed commits - there's not something like 17 commits in that PR it's difficult to see what's going on.

Even the topic/title of this ticket no longer seems to reflect what’s now being handled in this bug, since plain version numbers apparently work.

For now - I don’t think we want to require .extensions - but I’ve not yet seen the changes are that using it, if it’s purely optional then it may be ok - hopefully I'll have time to look at this when I get home tonight.

@bvella
Copy link
Contributor

bvella commented Jan 16, 2019

This is purely optional, and the plugin still works if the .mvn/extensions.xml file is not present.

The way it works is that an extension is allowed to replace built-in components if it is loaded from the extensions.xml file, and in this case, the class TilesProjectBuilder. As such, in cases where developers want to build tiles in the same reactor build using -pl, -am, -amd etc, they would add a folder .mvn at the root of the project and add the tiles plugin to the extensions.xml, and the tiles would be added as dependencies to the projects such that they are filtered and ordered in the project graph.

In any other case where tile dependencies make no difference, e.g. getting the tiles from a repository, there is no need for adding the extension and subsequently, TilesProjectBuilder is not used.

@talios
Copy link
Member

talios commented Jan 20, 2019

This should probably be raised under it's own issue now, since the change actually has nothing to do with "plain version numbers" as such - and include a change to the readme/docs.

I wonder - should the change make using .mvn/extensions.xml mandatory - i.e. a build failure IF there are multiple projects in the session ( aka a reactor ) and any of them happen to be tiles?

Any Maven 3.3.9 support changes should also be in their own issue/commit separate from this change.

@bvella
Copy link
Contributor

bvella commented Jan 21, 2019

separated maven 3.3.9 compatibility from extension to inject dependencies. We can continue the conversation in the pull request #91 if you want

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

No branches or pull requests

3 participants