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

Added rewriting the ${revision} for dependencies and removing the par… #22

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Tijs-2
Copy link
Contributor

@Tijs-2 Tijs-2 commented Jul 30, 2020

Hi Jean-Christophe,

I have found your library and used it to push a multi module project into my repository but it I missed two functions.

  • Removing the relative part of the parent pom
  • Rewriting the dependency $revision to the actual version.
    Which I both needed to use the pom for another project as a parent pom

Not sure if this is wanted but I needed itanyway and it seems to work, so I thought maybe it is a good addition.

Tijs

@coveralls
Copy link

Coverage Status

Coverage increased (+3.3%) to 78.333% when pulling ddddbe7 on Tijs-2:revision_for_dependencies into 3b46319 on jcgay:master.

@jcgay
Copy link
Owner

jcgay commented Jul 31, 2020

Thanks for your contribution!

Just to let you know in case you miss it but most of this extension functionality is now covered by https://maven.apache.org/maven-ci-friendly.html#install-deploy

For the added functionality, I don't really understand how the removal of relativePath in the parent section is linked to the ${revision} usage, I don't really feel it has its place here ? Or maybe you can describe me the structure of your projet ?

Replacing the ${revision} in the dependencies seems fine (we are already doing it for the dependencyManagement section). In most cases you should be able to use ${project.version} by the way. I'll merge it 👍

@Tijs-2
Copy link
Contributor Author

Tijs-2 commented Jul 31, 2020

First of all thanks for the quick reply.

I tried to create a structure, it is a bit complicated, and only a demo so not working and not real code but just to show some idea of the structure: https://github.com/Tijs-2/example

But what I wanted to archive is this:
You have a main pom in the main folder.

There are then 4 modules:
IntermediatePom --> Has the main as parent and contains general java build info.
JavaPom --> Has Intermediate pom as parent and contains the library as dependency. This pom is used by other projects that I build (mainly small services that use the library as a base.)
KotlinPom -->Has the main as parent and things needed to create Kotlin projects. Also has the library as dependency. So it is the same as the java pom but then for kotlin projects.
Library --> General code I use in multiple services.

Before this structure if I updated the main pom I also had to update the library which was a separate project. Then update both on a project that uses them. Also versions would be out of sync etc etc. With multiple small updates that becomes quite an annoyance.

Also the stuff on how to build the library and my java projects where copy/pasted into every projects and a change meant changing all those projects. So by setting them into this structure the library and other projects can share more "pom" code.


Versioning:
The idea is that if I include the latest Java or Kotlin pom the projects also get all info to build themselves and the latest version of the library.

The problem is that the ${revision} and ${project.version} needs to be replace otherwise you cant use it as a parent in another project. It will then try to use the library but with the version of the project that uses the pom as a parent.
Also the flattening library does not convert them correctly (mojohaus/flatten-maven-plugin#120 I think...)

There are also other problems in the flatten plugin which keep me from using it, it does too much and introduce problems like this: mojohaus/flatten-maven-plugin#152
Your module does exactly what I need except for the dependancy version update and relativePath and is easy to read and understand.


relativePath:
If you use a parent pom that is another module you need to specify a relativePath. My problem is that if you publish this pom with the relativePath into a central maven repository you cannot use it because that path does not exists.

I am not sure if it should be part of this project because of the name but uploading a pom with a relativePath makes it unusable and for me it was easier to add it to this library then to create a new library that only replaces the relativePath before uploading. And I think you always want this because a pom uploaded into a central maven repository cannot work with relativePaths (but I might be mistaken)


Hopefully this is a bit clear, I tried to split it into the two topics which hopefully helps to read my message...

Tijs

@jcgay
Copy link
Owner

jcgay commented Aug 15, 2020

Thanks for the example and the really clear message (and sorry for the late reply 😇).

Your multi-module structure seems quite common so that you shouldn't go in much trouble 😅.

I haven't use the flatten-maven-plugin much by myself but it seems to have room for improvement :)

Your problem with the relativePath seems odd to me, it should not a problem to publish such pom as the documentation states that the parent pom will be resolved from the local / remonte repository if it is not present on the filesystem: https://maven.apache.org/ref/3.0/maven-model/maven.html#class_parent
Can you add an example in https://github.com/Tijs-2/example ?

I'm merging the dependencies part 👍

@jcgay jcgay closed this in e3e1433 Aug 15, 2020
@jcgay
Copy link
Owner

jcgay commented Aug 15, 2020

Just reopening for the relativePath part 😇

@jcgay jcgay reopened this Aug 15, 2020
@Tijs-2
Copy link
Contributor Author

Tijs-2 commented Aug 18, 2020

I was testing again with the relativePath to create the example but I did not receive the issue again. So maybe it was a different problem than what I thought it was and fixed it at the same time that I removed the relativePath...

So for now I think this issue can be closed :)
And thank you for merging the dependency fix

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