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

Try to generate maven project skeleton from our dependencies #4194

Merged
merged 5 commits into from Jun 17, 2022

Conversation

ebarboni
Copy link
Contributor

@ebarboni ebarboni commented Jun 3, 2022

Try to generate a maven multimodule pom from externals libraries located in nbbuild/build/mavenpoms.

remove getmavencoordinate I introduced a year ago to debug nb-repository

On this generated folder I tried
mvn org.owasp:dependency-check-maven:7.1.0:check it could works (api rate limitation) some blocker due to too old dep.

was thinking to push the pseudo maven project to https://github.com/apache/netbeans-temp and put a dependatbot on it.

@ebarboni ebarboni marked this pull request as draft June 3, 2022 22:09
@mbien
Copy link
Member

mbien commented Jun 4, 2022

interesting. I am wondering if we could add an ant target which builds this temporary mega pom. After that we could use versions-maven-plugin and call versions:display-dependency-updates (it can be configured with a filter if needed).

This could be used by devs via CLI or maybe in manually triggered workflows / jenkins etc.

I don't think dependabot will be of much use since it can't translate the upgrade back to NetBeans' own dependency management system. Even if it could, it would cause too many PRs and too much CI load to be feasible in my opinion. NB has so many dependencies (many redundant on a per-module basis) that it wouldn't surprise me to get hundreds of PRs once its turned on.

@ebarboni
Copy link
Contributor Author

ebarboni commented Jun 7, 2022

I may be wrong but the default config for dependabot is for security dependencies so maybe less PR. On the nbm** repo I configured dependabot manually to get the version upgrade.

The generated pom are one parent pom + child pom for each external with maven coordinate. But it's not always the highest version the better we may need last upgrade in major. But both plugin can be run against the poms I guess

@ebarboni ebarboni requested a review from pepness June 8, 2022 12:24
@ebarboni
Copy link
Contributor Author

ebarboni commented Jun 8, 2022

@pepness does a lot of version upgrade maybe good to have his opinion.

you are right @mbien the PR dependencies based on maven would be useless in our case.

@mbien
Copy link
Member

mbien commented Jun 8, 2022

you are right @mbien the PR dependencies based on maven would be useless in our case.

I think they might be useful if it simply generates a list (just like the version plugin basically). This could give us a good overview about what is used where, point out EOL dependencies etc.

But the main "feature" of dependabot is that it is creating PRs, and those PRs run the tests. Devs can see ahead of time if a lib upgrade breaks the project. That won't work for us because we can't translate the maven versions back to NB's own dependency system (maybe we can, but thats extra work - might not be worth it).

However, I don't think this is bad at all! I believe having the ability to print all used versions and possible updates can be quite useful. Dependabot would have created most likely too much noise anyway for a project like NB.

@ebarboni
Copy link
Contributor Author

kind of a first approach. use mvn site site:stage on the folder.
to run successfully you need to comment on parent pom idehtmlvalidation,plafromlibsjavafx, nbbuild
it will generate site maybe resulting should be discusss on private ml

@ebarboni ebarboni marked this pull request as ready for review June 10, 2022 16:08
@mbien mbien changed the title Try to generate maven project squeleton from our dependencies Try to generate maven project skeleton from our dependencies Jun 14, 2022
@mbien mbien added the Maven [ci] enable "build tools" tests label Jun 14, 2022
@ebarboni
Copy link
Contributor Author

about to merge doing the mvn site or mvn site:stage will be left to dev on their box. Not sure is good to have the report "public"

@ebarboni
Copy link
Contributor Author

@mbien not sure it's maven alone maybe CI too will not change the NetBeans itself


private File mavenfolderFile;
/** JUnit-format XML result file to generate, rather than halting the build. */
public void setMavenFolder(File report) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks strange. The VerifyLibsAndLicenses task is pretty specificy. Why is this here? Could this be extracted into a seperate task?

Copy link
Contributor Author

@ebarboni ebarboni Jun 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it could be but to me the verify included the check for cve. Not an isssue to create new task just let me know.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer a separate task - as said generated a pom.xml has little in common with checking licenses (IMHO). They are as near as the already separate download task.

Copy link
Member

@mbien mbien left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks. looks good to me!

tested with:

$ ant verify-libs-and-licenses
...
$ tree nbbuild/build/mavenpoms/
nbbuild/build/mavenpoms/
├── pom.xml
├── cpplitecpplitedebugger
│   └── pom.xml
├── enterprisecloudoracle
│   └── pom.xml
├── enterprisej2eeapis
│   └── pom.xml
...
$ cd nbbuild/build/mavenpoms/
$ mvn org.owasp:dependency-check-maven:7.1.0:check
...
...

this couldn't resolve some references, but this is not the fault of the poms as far as i see

$ mvn versions:display-dependency-updates
....
INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  21.876 s

@mbien mbien added this to the NB15 milestone Jun 17, 2022
@mbien mbien added the build label Jun 17, 2022
@ebarboni ebarboni merged commit 19bb894 into apache:master Jun 17, 2022
@ebarboni
Copy link
Contributor Author

thks merging

@mbien
Copy link
Member

mbien commented Jun 17, 2022

you forgot to squash :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Maven [ci] enable "build tools" tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants