fedora-java / javapackages

Macros and scripts for Java packaging support
Other
7 stars 15 forks source link

Bring back the add_maven_depmap macro and make it a bit more intelligent #92

Closed fridrich closed 1 year ago

fridrich commented 2 years ago

I know that it is a bit of stretch. This are my patches that bring back the add_maven_depmap macro and make it a bit more intelligent as to generation of runtime dependencies. Not resolving them at compile time, so un-installable packages can happen, but still it is kind of better then not having any dependency generation at all. Added also a %mvn_install_pom that will resolve the pom variables and deps as much as one can and install a simple pom that is then usable with the %add_maven_depmap macro.

fridrich commented 2 years ago

Mikolaj, I know this is a bit big. But it should not impact anything if one does not use the %add_maven_depmap macro. We use it because we try to avoid build cycles as pest and I redid build of the whole stack until xmvn-tools by ant + add_maven_depmap. With this commit, there is a basic generation of dependencies for the resulting rpm package and I add a python script that resolves the pom.xml file using the structure of parents (as far as it can) and installs that file as a dependency only pom file.

mizdebsk commented 2 years ago

I'll review this. I don't mind re-adding %add_maven_depmap if it is still useful. It was deprecated and then removed because other newer macros and tools (%mvn_artifact and %mvn_install) can be used to achieve the same. These depend on XMvn or javapackages-bootstrap - "In a nutshell, Java Packages Bootstrap (JPB) is a standalone build of all Java software packages that are required for Java Packages Tools (JPT) to work."

fridrich commented 2 years ago

Yes, I saw your javapackages-bootstrap and in principle it is great idea. Just that we rebuild the packages differently then you do. Any package that is in openSUSE:Tumbleweed comes from a development project in OBS. The packages in development projects rebuild any time something in their dependencies changes. And the rebuilds are transitive. So, I cannot set a flag "bootstrap" and then once it rebuilt, cancel it. That is why some of the packages in the food-chain, I regenerate ant build system, adapt it, and build them with ant. There the add_maven_depmap is useful to generate the metadata. But for that to be really useful in more complex packages, I have to at least interpolate the information in the pom files project-wide. That is why I wrote that install_pom.py script. If xmvn-resolve is present, it can actually descend all the parents for information, but generally, it is enough to do that project-wise, since I only need to know dependencies and not which plugin and what profile is applied at which time.

codecov-commenter commented 2 years ago

Codecov Report

Merging #92 (228bff1) into master (a2826a4) will increase coverage by 1.50%. The diff coverage is 96.31%.

@@            Coverage Diff             @@
##           master      #92      +/-   ##
==========================================
+ Coverage   89.69%   91.20%   +1.50%     
==========================================
  Files          43       45       +2     
  Lines        3009     3389     +380     
==========================================
+ Hits         2699     3091     +392     
+ Misses        310      298      -12     
Impacted Files Coverage Δ
java-utils/install_pom.py 95.03% <95.03%> (ø)
java-utils/maven_depmap.py 97.26% <97.26%> (ø)

... and 7 files with indirect coverage changes

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

fridrich commented 2 years ago

I think I finished here. My internal tests show that this will bring the test coverage to 91% Both maven_depmap.py and install_pom.py have a coverage of 97%. The rest might be paths that actually will never be hit, but I am not 100% sure, so leaving them in.

fridrich commented 2 years ago

Mikolaj, are you around, or on vacation?

fridrich commented 2 years ago

Any chance to review this one?

fridrich commented 1 year ago

Ping?

mizdebsk commented 1 year ago

I'm sorry this review has been waiting for so long. Thanks for the nag, I'll try to prioritize it.

fridrich commented 1 year ago

reping :)

fridrich commented 1 year ago

ping ;)

mizdebsk commented 1 year ago

Everything looks good, thank you for your contribution. I didn't test the new macro/script, but I assume it works for you. I will try to make a new release soon.