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

GH-40515: [Java] Bump org.apache.maven dependencies from 3.3.9 to 3.8.7 #40514

Merged
merged 23 commits into from
Mar 22, 2024

Conversation

ianmcook
Copy link
Member

@ianmcook ianmcook commented Mar 13, 2024

  • Updates the Maven version required in /java/maven/module-info-compiler-maven-plugin to 3.8.7 which addresses vulnerabilities identified by https://deps.dev/maven/org.apache.maven%3Amaven-core/3.3.9.
  • Updates .env to use Maven version 3.8.7.
  • Bumps older versions of Maven to 3.8.7 in ci/docker/*.dockerfile
  • Updates the release verification instructions to say that Maven 3.8.7 is required.

@ianmcook
Copy link
Member Author

@vibhatha could you take a look at this please? I'm not sure if this will work or what else we need to do.

@kou
Copy link
Member

kou commented Mar 13, 2024

It seems that we have more places that refer Maven version. For example:

arrow/.env

Line 68 in bd3fab4

MAVEN=3.6.3

So this may not be a MINOR change. Could you open an issue for this?

@ianmcook ianmcook changed the title MINOR: [Java] Bump org.apache.maven dependencies from 3.3.9 to 3.8.8 to in /java/maven/module-info-compiler-maven-plugin GH-40515: [Java] Bump org.apache.maven dependencies from 3.3.9 to a higher version Mar 13, 2024
Copy link

⚠️ GitHub issue #40515 has been automatically assigned in GitHub to PR creator.

@ianmcook
Copy link
Member Author

ianmcook commented Mar 13, 2024

Could you open an issue for this?

#40515

@lidavidm
Copy link
Member

 > [2/2] RUN mamba install -q -y         maven=3.8.8         openjdk=11         jpype1 &&     mamba clean --all:
11.63 Could not solve for environment specs
11.63 The following package could not be installed
11.63 └─ maven 3.8.8**  does not exist (perhaps a typo or a missing channel).

I think you'll also have to pick a version of Maven in conda-forge

@lidavidm
Copy link
Member

lidavidm@debian ~> mamba repoquery search 'maven'                                                                                                                                                                
 Name  Version Build      Channel             
───────────────────────────────────────────────
 maven 3.9.6   ha770c72_0 conda-forge/linux-64
 maven 3.9.5   ha770c72_0 conda-forge/linux-64
 maven 3.9.4   ha770c72_0 conda-forge/linux-64
 maven 3.9.3   ha770c72_0 conda-forge/linux-64
 maven 3.9.2   ha770c72_1 conda-forge/linux-64
 maven 3.9.2   ha770c72_2 conda-forge/linux-64
 maven 3.9.2   ha770c72_3 conda-forge/linux-64
 maven 3.9.1   ha770c72_0 conda-forge/linux-64
 maven 3.9.0   ha770c72_0 conda-forge/linux-64
 maven 3.8.7   ha770c72_0 conda-forge/linux-64
 maven 3.8.6   ha770c72_0 conda-forge/linux-64
 maven 3.8.5   ha770c72_0 conda-forge/linux-64
 maven 3.6.3   ha770c72_0 conda-forge/linux-64
 maven 3.6.0   0          conda-forge/linux-64
 maven 3.5.0   0          conda-forge/linux-64
 maven 3.3.9   0          conda-forge/linux-64

@ianmcook
Copy link
Member Author

Should I try 3.9.6?
https://maven.apache.org/docs/history.html

@lidavidm
Copy link
Member

I think that's fine so long as it's installable in all of our different CI environments? (e.g. does Debian or something else use a lower version?)

@ianmcook
Copy link
Member Author

I'll try 3.9.6 first and drop to 3.8.7 if we see failures

@ianmcook
Copy link
Member Author

Looks like 3.9.6 would require some more extensive changes so I'll first drop to 3.8.7 and see if that works.

@kou
Copy link
Member

kou commented Mar 14, 2024

@github-actions crossbow submit -g java

Copy link

Revision: 2b736d1

Submitted crossbow builds: ursacomputing/crossbow @ actions-aa61a793e5

Task Status
java-jars GitHub Actions
verify-rc-source-java-linux-almalinux-8-amd64 GitHub Actions
verify-rc-source-java-linux-conda-latest-amd64 GitHub Actions
verify-rc-source-java-linux-ubuntu-20.04-amd64 GitHub Actions
verify-rc-source-java-linux-ubuntu-22.04-amd64 GitHub Actions
verify-rc-source-java-macos-amd64 GitHub Actions

@vibhatha
Copy link
Collaborator

@ianmcook I will take a look at this.
I also have a PR here: #40149 which is not the same but could be related.

@vibhatha
Copy link
Collaborator

Also I attempted this before: #39451 but we closed it at that time since it was not needed.

Shall I reopen this and work? What's the best?

cc @lidavidm @ianmcook

@ianmcook
Copy link
Member Author

ianmcook commented Mar 14, 2024

Thanks @vibhatha. I think what's important here is:

  • We do not want to require Arrow Java developers use old, unpatched Maven versions.
  • We also do not necessarily want to require all Arrow Java environments (including our CI environments) to have the newest Maven version. (@lidavidm made this comment here )

So how can we achieve that? Can we set minimum versions but allow environments to have newer versions?

In a pom.xml file, does <version>3.3.9</version> mean that we require version 3.3.9 exactly, or version 3.3.9 or higher? (I think it means "or higher" but I'm seeing conflicting information about this.)

Should we be using version ranges like <version>[3.3.9,)</version>?

@lidavidm
Copy link
Member

It looks like the problem is that a few builds have too old a Maven. Could we have the verification script (or the build setup) download a later Maven instead of relying on the OS packaged Maven?

@ianmcook
Copy link
Member Author

It looks like the problem is that a few builds have too old a Maven. Could we have the verification script (or the build setup) download a later Maven instead of relying on the OS packaged Maven?

It looks like everything is wired up to do that (at least on Linux-like environments) and we just need to set USE_CONDA=1?

@ianmcook
Copy link
Member Author

ianmcook commented Mar 14, 2024

Also I attempted this before: #39451 but we closed it at that time since it was not needed.

Shall I reopen this and work? What's the best?

@vibhatha Maybe let's use this PR to try to get to 3.8.7, then maybe later you can reopen #39451 to try to get to 3.9.6?

@vibhatha
Copy link
Collaborator

@ianmcook can I get permission to commit to this branch?

@ianmcook
Copy link
Member Author

@ianmcook can I get permission to commit to this branch?

yes, invite sent. thanks!

@vibhatha
Copy link
Collaborator

@ianmcook can I get permission to commit to this branch?

yes, invite sent. thanks!

Thanks @ianmcook

@vibhatha
Copy link
Collaborator

@github-actions crossbow submit -g java

@github-actions github-actions bot added awaiting changes Awaiting changes awaiting change review Awaiting change review and removed awaiting changes Awaiting changes awaiting change review Awaiting change review labels Mar 21, 2024
dev/release/verify-release-candidate.sh Outdated Show resolved Hide resolved
dev/release/verify-release-candidate.sh Outdated Show resolved Hide resolved
ianmcook and others added 2 commits March 21, 2024 16:27
Co-authored-by: Sutou Kouhei <[email protected]>
Co-authored-by: Sutou Kouhei <[email protected]>
@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Mar 21, 2024
@ianmcook
Copy link
Member Author

@github-actions crossbow submit -g java

Copy link

Revision: 9eea160

Submitted crossbow builds: ursacomputing/crossbow @ actions-2f83250be0

Task Status
java-jars GitHub Actions
verify-rc-source-java-linux-almalinux-8-amd64 GitHub Actions
verify-rc-source-java-linux-conda-latest-amd64 GitHub Actions
verify-rc-source-java-linux-ubuntu-20.04-amd64 GitHub Actions
verify-rc-source-java-linux-ubuntu-22.04-amd64 GitHub Actions
verify-rc-source-java-macos-amd64 GitHub Actions

Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

+1 after we resolve #40514 (comment) .

@github-actions github-actions bot added awaiting changes Awaiting changes awaiting merge Awaiting merge and removed awaiting change review Awaiting change review awaiting changes Awaiting changes labels Mar 21, 2024
@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting merge Awaiting merge labels Mar 21, 2024
@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Mar 21, 2024
@kou kou merged commit 5181791 into apache:main Mar 22, 2024
55 checks passed
@kou kou removed the awaiting change review Awaiting change review label Mar 22, 2024
Copy link

After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 5181791.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 2 possible false positives for unstable benchmarks that are known to sometimes produce them.

@ianmcook ianmcook deleted the bump_maven branch March 22, 2024 14:17
lriggs pushed a commit to lriggs/arrow that referenced this pull request Mar 22, 2024
…to 3.8.7 (apache#40514)

- Updates the Maven version required in `/java/maven/module-info-compiler-maven-plugin` to 3.8.7 which addresses vulnerabilities identified by https://deps.dev/maven/org.apache.maven%3Amaven-core/3.3.9.
- Updates `.env` to use Maven version 3.8.7.
- Bumps older versions of Maven to 3.8.7 in `ci/docker/*.dockerfile`
- Updates the release verification instructions to say that Maven 3.8.7 is required.

-----
* GitHub Issue: apache#40515

Lead-authored-by: Ian Cook <[email protected]>
Co-authored-by: Vibhatha Abeykoon <[email protected]>
Co-authored-by: vibhatha <[email protected]>
Co-authored-by: Sutou Kouhei <[email protected]>
Co-authored-by: Dane Pitkin <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants