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

Use exact maven versions #8337

Closed
nkavian opened this issue Jul 22, 2021 · 9 comments
Closed

Use exact maven versions #8337

nkavian opened this issue Jul 22, 2021 · 9 comments

Comments

@nkavian
Copy link

nkavian commented Jul 22, 2021

What version of gRPC-Java are you using?

1.38.0

What is your environment?

Maven

What did you expect to see?

Developers and DevOps people that build CI/CD pipelines expect builds to be reproducible and deterministic. If we build a project today or next year (assuming the same exact commit id), we expect the same binary to be built. That is not the case with the grpc maven artifacts.

For example, snapshots are discouraged from being used in production builds because they can be overwritten with newer code or completely deleted; either changing application behavior or breaking the build.

What did you see instead?

When building the project using maven it will reach out across the network to check for a newer version of grpc to build with. This opens the project to builds that are not deterministic.

Steps to reproduce the bug

I have a maven project that uses grpc-okhttp, and I execute mvn clean install. Observe the output contains

[INFO] Building Project 1.0.0
[INFO] --------------------------------[ jar ]---------------------------------
Downloading from atlassian-public: https://packages.atlassian.com/mvn/maven-external/io/grpc/grpc-api/maven-metadata.xml
Downloading from sourceforge-releases: https://oss.sonatype.org/content/repositories/sourceforge-releases/io/grpc/grpc-api/maven-metadata.xml
Downloading from central: https://repo.maven.apache.org/maven2/io/grpc/grpc-api/maven-metadata.xml
Downloaded from central: https://repo.maven.apache.org/maven2/io/grpc/grpc-api/maven-metadata.xml (1.7 kB at 3.3 kB/s)
Downloaded from atlassian-public: https://packages.atlassian.com/mvn/maven-external/io/grpc/grpc-api/maven-metadata.xml (1.7 kB at 2.6 kB/s)

Expected Resolution

Please consider updating this line of code so that it stops wrapping the version with [ ] and use only specific version numbers:

https://github.com/grpc/grpc-java/blob/master/build.gradle#L411

core.version*.value = "[" + core.version.text() + "]"

Instead of [1.38.0] (which causes the network activity), the unwrapped value 1.38.0 is the expected output.

@ejona86
Copy link
Member

ejona86 commented Jul 27, 2021

You talk about reproducability/deterministic but then talk about network access. The two are a bit different here.

The [] syntax means "only this version." The selected version is thus much more deterministic than without the [] as bare versions allows upgrades and downgrades in Maven. We use the [] syntax because choosing a different version is highly likely to be broken due to internal APIs. We use a similar approach for Netty, although you only see that if using unshaded-grpc-netty.

The network access is because the square brackets and parentheses define compatible version ranges so Maven goes and looks up the available versions. With our specific usage though, the results are deterministic. The network access is quite unfortunate, but there's not many alternatives.

@nkavian
Copy link
Author

nkavian commented Jul 27, 2021

Yeah, I did make a bit of a leap without connecting the dots in my explanation.

When a non-bracketed dependency version used in pom.xml matches the dependency stored in the local maven repository, it doesn't hit the network looking for an update, it simply uses what exists in the local maven repository.

When a bracketed version is used, such as [1.38.0,1.40.0,1.50.0], maven needs to know what exists in the remote maven repository and hits the network. If the latter two versions don't exist yet, maven would know to select 1.38.0 for the time being. Apparently, maven is not smart enough when a single list entry exists.

So:

  • If you're using a list of versions, this ticket would be to request considering making builds deterministic and adjust to using a single non-bracketed version.
  • If you're always using a single version, could you please consider using it without brackets to help avoid the unnecessary network activity.
  • If the project insists on using brackets, could it at least consider upgrading the gradle code to only use brackets when there is more than 1 version.

I'm not sure I got your example about choosing versions due to broken APIs. Sounds like what you are saying is that you do prefer to use a list of versions in case something is broken and you want to automatically fix things. That's kind of my point, developers should be forced into version upgrades unless they've chosen to.

Feel free to close the ticket if it's not going to be fixed.

@ejona86
Copy link
Member

ejona86 commented Jul 27, 2021

If you're always using a single version, could you please consider using it without brackets to help avoid the unnecessary network activity.

As I said, the two aren't really equivalent. Does mvn --offline not work for you? That seems the ideal flag for hermetic environments that you don't want accessing the network.

If the project insists on using brackets, could it at least consider upgrading the gradle code to only use brackets when there is more than 1 version.

We don't want to use actual ranges. We do want the version used to be reproducable. The only reason we allowed ourselves to use [] is because it was a single version.

Sounds like what you are saying is that you do prefer to use a list of versions in case something is broken and you want to automatically fix things.

It's more that we have internal classes that are reused across modules, but they have no API stability. We encourage keeping the versions of the various grpc components in sync (all the same version), but some things that we know are using those internal APIs we pin the versions to reduce chances a user gets bitten by version skew.

@nkavian
Copy link
Author

nkavian commented Jul 27, 2021

We don't want to use actual ranges. We do want the version used to be reproducable. The only reason we allowed ourselves to use [] is because it was a single version.

Thanks for clarifying! If a single version is desired, then a bracket isn't necessary. Your builds will work exactly the same irrespective of API stability issues if you remove the brackets. When you set a non-bracketed version, it's still pinned to that version. For example, here is the pom.xml and no dependency except gRPC uses brackets.

I'm not suggesting to fully remove the version, just the brackets.. If you remove the version completely, then yeah, that would be a problem and it would be 'unpinned'.

If I missed your point about API stability, my apologies. Don't want to keep replying and seem like I'm pressing.

@ejona86
Copy link
Member

ejona86 commented Jul 27, 2021

Thanks for clarifying! If a single version is desired, then a bracket isn't necessary. Your builds will work exactly the same irrespective of API stability issues if you remove the brackets.

Nope. They are treated differently by Maven. I made the change on purpose. 90db93b

A bare version suggests a version, but upgrading and downgrading are allowed. It is essentially unrestricted. A []-encoded version pins a version.

To demonstrate, take a look at this pom:

<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
  xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
  <modelVersion>4.0.0</modelVersion>
  <groupId>io.grpc</groupId>
  <artifactId>examples</artifactId>
  <packaging>jar</packaging>
  <version>1.38.1</version>
  <name>examples</name>

  <dependencies>
    <dependency>
      <groupId>io.grpc</groupId>
      <artifactId>grpc-netty</artifactId>
      <version>1.39.0</version>
      <scope>runtime</scope>
    </dependency>
    <dependency>
      <groupId>io.grpc</groupId>
      <artifactId>grpc-netty-shaded</artifactId>
      <version>1.38.1</version>
      <scope>runtime</scope>
    </dependency>
  </dependencies>
</project>

If you try to build, you'll get an error because of the version ranges. Without the version ranges this would have compiled (but may have behaved strange at runtime):

$ mvn compile
[INFO] Scanning for projects...
[INFO] 
[INFO] --------------------------< io.grpc:examples >--------------------------
[INFO] Building examples 1.38.1
[INFO] --------------------------------[ jar ]---------------------------------
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  0.192 s
[INFO] Finished at: 2021-07-27T15:56:40-07:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal on project examples: Could not resolve dependencies for project io.grpc:examples:jar:1.38.1: Failed to collect dependencies for io.grpc:examples:jar:1.38.1: Could not resolve version conflict among [io.grpc:grpc-netty:jar:1.39.0 -> io.grpc:grpc-core:jar:[1.39.0,1.39.0], io.grpc:grpc-netty-shaded:jar:1.38.1 -> io.grpc:grpc-core:jar:[1.38.1,1.38.1]] -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/DependencyResolutionException

Or consider this pom (grpc-core instead of grpc-netty as before):

<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
  xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
  <modelVersion>4.0.0</modelVersion>
  <groupId>io.grpc</groupId>
  <artifactId>examples</artifactId>
  <packaging>jar</packaging>
  <version>1.38.1</version>
  <name>examples</name>

  <dependencies>
    <dependency>
      <groupId>io.grpc</groupId>
      <artifactId>grpc-core</artifactId>
      <version>1.39.0</version>
      <scope>runtime</scope>
    </dependency>
    <dependency>
      <groupId>io.grpc</groupId>
      <artifactId>grpc-netty-shaded</artifactId>
      <version>1.38.1</version>
      <scope>runtime</scope>
    </dependency>
  </dependencies>
</project>

The normal version resolution order would have chosen grpc-core 1.39.0, since that is the first in the breadth-first search. But maven is selecting 1.38.1 because it has to select a version in the "range":

$ mvn dependency:tree
[INFO] Scanning for projects...
[INFO] 
[INFO] --------------------------< io.grpc:examples >--------------------------
[INFO] Building examples 1.38.1
[INFO] --------------------------------[ jar ]---------------------------------
[INFO] 
[INFO] --- maven-dependency-plugin:2.8:tree (default-cli) @ examples ---
[INFO] io.grpc:examples:jar:1.38.1
[INFO] +- io.grpc:grpc-core:jar:1.38.1:runtime
[INFO] |  +- io.grpc:grpc-api:jar:1.38.1:runtime (version selected from constraint [1.38.1,1.38.1])
[INFO] |  |  \- io.grpc:grpc-context:jar:1.38.1:runtime
[INFO] |  +- com.google.code.gson:gson:jar:2.8.6:runtime
[INFO] |  +- com.google.android:annotations:jar:4.1.1.4:runtime
[INFO] |  +- org.codehaus.mojo:animal-sniffer-annotations:jar:1.19:runtime
[INFO] |  +- com.google.errorprone:error_prone_annotations:jar:2.4.0:runtime
[INFO] |  +- com.google.guava:guava:jar:30.1-android:runtime
[INFO] |  |  +- com.google.guava:failureaccess:jar:1.0.1:runtime
[INFO] |  |  +- com.google.guava:listenablefuture:jar:9999.0-empty-to-avoid-conflict-with-guava:runtime
[INFO] |  |  +- org.checkerframework:checker-compat-qual:jar:2.5.5:runtime
[INFO] |  |  \- com.google.j2objc:j2objc-annotations:jar:1.3:runtime
[INFO] |  +- io.perfmark:perfmark-api:jar:0.23.0:runtime
[INFO] |  \- com.google.code.findbugs:jsr305:jar:3.0.2:runtime
[INFO] \- io.grpc:grpc-netty-shaded:jar:1.38.1:runtime
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  0.735 s
[INFO] Finished at: 2021-07-27T15:59:48-07:00
[INFO] ------------------------------------------------------------------------

All that said, I did that before BOMs were really a thing. The behavior is different with BOMs.

@nkavian
Copy link
Author

nkavian commented Jul 28, 2021

Thanks for your patience, I followed your link into the past work you did 5 years ago and thankfully you were thorough in documenting your rationale. It was helpful to read it and understand your context. This link explained the exact situation.

The solution you applied solves your issue, and I've never seen versions used that way. I'm more accustomed to seeing parent poms that leverage dependencyManagement to manage versions and usage of the enforcer plugin to enforce convergence. Here is an article of a similar scenario. For example, if you did not use brackets, the enforcer plugin would detect multiple versions of the same dependency and break the build.

I can also understand that it's even more complicated since the gradle file is creating a pom file on the fly, so it's perhaps not as convenient to use dependencyManagement and the enforcer plugin.

To provide a contrast, the brackets help address an internal versioning of gRPC components, but as a consumer of gRPC I'm still going to struggle with versioning no matter what. For example, gRPC is using the guava library version 30.1, while a different library I'm using prefers 28.0.. The best practice I have employed on my side is that I still use the enforcer plugin to find these convergence issues.

@ejona86
Copy link
Member

ejona86 commented Jul 28, 2021

This link explained the exact situation.

Earlier I tried searching for official documentation on the matter to share with you but failed. I just stumbled on it: https://maven.apache.org/pom.html#Dependency_Version_Requirement_Specification .

usage of the enforcer plugin to enforce convergence.

We are strongly against dependencyConvergence and would discourage our Maven users from using it. It has no problem with downgraded package versions and it is very brittle to use when upgrading dependencies as part of normal maintenance. If you use excludes to manage the versions, you will be hiding versions from Maven asking for future problems. We recommend requireUpperBoundDeps, which doesn't have these problems.

I can also understand that it's even more complicated since the gradle file is creating a pom file on the fly, so it's perhaps not as convenient to use dependencyManagement and the enforcer plugin.

Gradle doesn't need it at all, and specifying dependencyManagement in our pom files does nothing for our users from what I've seen. I've not looked into it deeply though. Gradle does have a function to do the equivalent of dependencyConvergence and we were using it until recently (#8238) due to lack of alternative to avoid Maven's broken dependency resolution.

For example, gRPC is using the guava library version 30.1, while a different library I'm using prefers 28.0

Libraries must not use @Beta APIs in Guava. I've yet to find an instance of dependency hell with Guava where Beta APIs were not involved. If the different library is OSS, I'd suggest filing a bug against the library.

@nkavian
Copy link
Author

nkavian commented Jul 28, 2021

Thanks for finding and sharing that dependency version link. I understand more clearly your point about soft/hard dependencies and the dependency tree. I also appreciate requireUpperBoundDeps and I'll try to implement it as well, it looks like the right thing to do.

You've convinced me that the brackets do serve a functional and required purpose. I'll work around the network issue using a workaround on my end. I still don't want it to output the network access during builds.

@ejona86
Copy link
Member

ejona86 commented Jul 28, 2021

You've convinced me that the brackets do serve a functional and required purpose.

I can imagine one day they may cause more pain than gain. For example, BOMs have reduced the likelihood of issues and Gradle users don't seemed to have suffered very often, given the rate of user reports. But yes, it is providing some value.

I'll work around the network issue using a workaround on my end. I still don't want it to output the network access during builds.

Do keep in touch if you continue to suffer and if --offline doesn't work for you. Your use-case is also legit. We just get to continue weighing the costs/benefits.

Closing as it seems we've come to an understanding, but if something changes comment and we can reopen.

@ejona86 ejona86 closed this as completed Jul 28, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants