setup-java feat: added support to pom.xml and build.gradle

Description: The idea of this PR is to extend the java-version-file to support pom.xml, because for now it only supports the java-version-file created by jEnv

Related issue: Add link to the related issue.

Check list:

[x] Mark if documentation changes are required.
[x] Mark if tests were added or updated to cover the changes.

Jan 08 '23 19:01 augustomelo

This PR is still pending:

[x] Add unit e2e test to cover the other scenarios
[x] Add documentation

Jan 08 '23 19:01 augustomelo

hi @augustomelo thank you for the PR, we will take a look at it

Jan 11 '23 11:01 panticmilos

Hello @panticmilos could you approve the workflow run ? I tested it locally with act and it was fine, just wanted to sure that I didn't break anything

Jan 17 '23 16:01 augustomelo

Hey, regarding the failing workflows, I believe there is something strange with them, because for example the code scanning:

GitHub Code Scanning / CodeQL complains in a file that is generated dist/cleanup/index.js
- 97022: if (util_1.isObject(p1) || (util_1.isString(p1) && (/^\s*</.test(p1) || /^\s*[\{\[]/.test(p1) || /^(\s*|(#.*)|(%.*))*---/.test(p1)))) {
- 97795: else if (p1 !== null && /^(\s*|(#.*)|(%.*))*---/.test(p1))

I believe this is coming from the xmlbuilder2 when I create the object from the string (here), should I change the library ? Because I don't think I have any control over it.

The other workflows that failed are:

Which are failing during the install dependencies step, it could be because the dependency that I added: xpath which has the same license as the one xmlbuilder2 (MIT).

Not sure how to proceed now, @panticmilos do you have any ideas here ?

Jan 22 '23 20:01 augustomelo

hi @augustomelo, and sorry for the delayed response. We had some chats about this PR, and we came to the conclusion that the 'xpath' is probably the one that is causing the issues. Now, we were wondering could the same goal be achieved without 'xpath'?

Also can you please rebase to the main branch?

Mar 23 '23 12:03 panticmilos

Hey @panticmilos thanks for the reply! Sure thing I will have a look on it once I have the time 😄

Mar 23 '23 20:03 augustomelo

hey @panticmilos did a refactor using regex, unfortunately I had to remove a way that the java version is retrieved from the maven-compiler-plugin, because I don't think it will reliably identify the java version

Apr 08 '23 12:04 augustomelo

Hey @augustomelo, first of all thanks for the PR, if this didn't exist I would have created my own ;)

But I wonder why you chose to parse the version via regex as that approach obviously limited. This project already has xmlbuilder2 as a dependency which can also be used for parsing XML. For example we could do this:

const doc = create(pomContentAsAString); // create function from xmlbuilder2
const mvnVersionNode = doc.find(n => n.node.nodeName === "maven.compiler.source", false, true);
const sourceVersion = mvnVersionNode.first().toString();

This way we can have a more robust implementation and support using the version specified in maven-compiler-plugin.

Are you still willing to work on this @augustomelo? Otherwise I can create a new PR.

@panticmilos has this feature request been discussed? Are you willing to merge something like this?

Mar 03 '24 16:03 tim-we

hey @tim-we, thanks for the review and I much appreciate the suggestion the solution is much better now!

I am not sure why, but when I read the xmlbuilder2 I understood that it was not possible to create the xml object from a string, therefore I used the xpath and then moved to using regex.

@panticmilos when you give a green light on this I will fix the merge conflicts

edit:

added last paragraph

Mar 04 '24 08:03 augustomelo

Hey @augustomelo, I made a PR to your fork to add support for versioning via maven-compiler-plugin. Please have a look. There are also some minor things we should address before this gets merged, such as variable names not being accurate anymore after the refactor. If this progresses any further I'd might do another code review.

Apr 13 '24 10:04 tim-we

hey @tim-we I didn't forgot about this, when I had the time I will apply the changes. I was thinking about also add support to gradle, but I currently don't know how to achieve that besides using RegEx, do you have any ideas ?

Apr 25 '24 10:04 augustomelo

No I actually don't know that much about gradle (currently learning). But I'm not aware of any parsers if thats what you mean.

Apr 25 '24 20:04 tim-we

hey @tim-we refactored the code to align with your comments, feel free to resolve the conversation.

Added the support to builld. gradle using regex.

I didn't do the merge, because I am not sure if this PR is going to be merged

Apr 27 '24 11:04 augustomelo

hey @tim-we I had to do some changes on the code since the testes were failing in correctly identifying the java version, now they seem fine, the failure is now not being able to find the version to download

@panticmilos and @IvanZosimov any new if this is going to be merged? if not I will not bother rebasing it for now

Apr 29 '24 16:04 augustomelo