Uncap remaining dependencies#3073
Conversation
There was a problem hiding this comment.
⚠️ Attention Required: Lockfile Detected
This pull request contains modifications to one or more *.lockfile files. Please confirm that you have run update_dependency.sh to push new dependencies to the private repo.
Someone with Admin role must manually dismiss this review before merging.
weiminyu
left a comment
There was a problem hiding this comment.
@weiminyu made 1 comment.
Reviewable status: 0 of 10 files reviewed, 1 unresolved discussion (waiting on CydeWeys).
GEMINI.md line 61 at r1 (raw file):
3. Run `./gradlew clean build` to exhaustively compile and run all tests against the new transitive dependency tree. If there are compilation or test errors due to breaking API changes or output formatting changes in the new dependency version, your primary goal is to **fix our codebase** to be compliant with the new dependency version. Only revert the dependency bump or cap the version if the required code changes are prohibitively complex or outside the scope of the current task. 4. Only once the build passes and `git status` shows modified lockfiles, commit the `dependencies.gradle` and lockfile changes. 5. ONLY after the local changes are committed and verified, execute the internal script `/google/src/head/depot/google3/domain/registry/tools/update_dependency.sh`. This ensures you don't corrupt the upstream remote artifact cache with broken or missing lockfiles.
If this script causes new changes to some lockfiles, commit them locally and push to upstream.
Code quote:
update_dependency.sh
CydeWeys
left a comment
There was a problem hiding this comment.
@CydeWeys made 1 comment.
Reviewable status: 0 of 10 files reviewed, 1 unresolved discussion (waiting on weiminyu).
GEMINI.md line 61 at r1 (raw file):
Previously, weiminyu (Weimin Yu) wrote…
If this script causes new changes to some lockfiles, commit them locally and push to upstream.
In step 4 it already says to commit the lockfile changes? Also we should never be pushing to upstream (the google/nomulus repo), as that has branch protection rules that only allows merges via PRs. If by upstream you meant your local fork (which I have configured as "origin"), then that's also something we don't want the AI doing automatically, or at least I don't. Elsewhere in this file we have this directive, which I think is a good one:
- No Unprompted Pushing: You MUST NEVER push your changes to a remote repository (e.g.,
git push origin ...) unless the user VERY EXPLICITLY instructs you to do so. If you have finished a task, simply state that it is committed locally and ready for review. Do not assume pushing is desired.
CydeWeys
left a comment
There was a problem hiding this comment.
@CydeWeys made 1 comment.
Reviewable status: 0 of 10 files reviewed, 1 unresolved discussion (waiting on weiminyu).
GEMINI.md line 61 at r1 (raw file):
Previously, CydeWeys (Ben McIlwain) wrote…
In step 4 it already says to commit the lockfile changes? Also we should never be pushing to upstream (the google/nomulus repo), as that has branch protection rules that only allows merges via PRs. If by upstream you meant your local fork (which I have configured as "origin"), then that's also something we don't want the AI doing automatically, or at least I don't. Elsewhere in this file we have this directive, which I think is a good one:
- No Unprompted Pushing: You MUST NEVER push your changes to a remote repository (e.g.,
git push origin ...) unless the user VERY EXPLICITLY instructs you to do so. If you have finished a task, simply state that it is committed locally and ready for review. Do not assume pushing is desired.
There are also other steps in these instructions and the PR polisher skill to check for unstaged changes in the working tree prior to declaring done.
|
Previously, CydeWeys (Ben McIlwain) wrote…
Occasionally when you update locks a second time, new versions show up. Probably coinciding with new publication, but it happens more often than you'd think. |
There was a problem hiding this comment.
⚠️ Attention Required: Lockfile Detected
This pull request contains modifications to one or more *.lockfile files. Please confirm that you have run update_dependency.sh to push new dependencies to the private repo.
Someone with Admin role must manually dismiss this review before merging.
CydeWeys
left a comment
There was a problem hiding this comment.
PTAL
@CydeWeys made 2 comments.
Reviewable status: 0 of 14 files reviewed, 1 unresolved discussion (waiting on weiminyu).
GEMINI.md line 61 at r1 (raw file):
Previously, weiminyu (Weimin Yu) wrote…
Occasionally when you update locks a second time, new versions show up. Probably coinciding with new publication, but it happens more often than you'd think.
Done.
CydeWeys
left a comment
There was a problem hiding this comment.
Now with some more changes to the sql Integration test suite build target.
@CydeWeys made 1 comment.
Reviewable status: 0 of 14 files reviewed, 1 unresolved discussion (waiting on weiminyu).
There was a problem hiding this comment.
⚠️ Attention Required: Lockfile Detected
This pull request contains modifications to one or more *.lockfile files. Please confirm that you have run update_dependency.sh to push new dependencies to the private repo.
Someone with Admin role must manually dismiss this review before merging.
weiminyu
left a comment
There was a problem hiding this comment.
@weiminyu reviewed 16 files and all commit messages, and resolved 1 discussion.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on CydeWeys).
This commit relaxes the upper bounds on several dependencies that were previously hardcapped to specific versions: - com.google.protobuf to [3.25.5,) and [3.17.3,) - org.apache.beam to [2.72.0,) - io.github.ss-bhatt to [1.0.0,) - io.protostuff to [1.8.0,) - redis.clients:jedis to [7.4.1,) - org.junit.jupiter and org.junit.platform to [5.6.2,) and [1.6.2,) - org.jcommander to [2.0,) - org.jline to [3.0,) - jakarta.servlet to [6.0,) Upgrading to the modern versions of jline introduced a breaking change where DefaultParser().parse(line, line.length()) strips trailing spaces when using the default ParseContext.UNSPECIFIED. This caused the autocompletion to misbehave and tests to fail. This commit fixes ShellCommandTest.java by explicitly passing ParseContext.COMPLETE when parsing test strings to perfectly mimic the real-world JLine completion context. Additionally, SqlIntegrationTestSuite was migrated to JUnit 5's @suite annotation, fixing a NoClassDefFoundError introduced by uncapping the JUnit Platform dependencies, and the test suite was re-integrated into the standard :build lifecycle. The following dependencies remain explicitly capped: 1. Hibernate & Jakarta Persistence (Blocked by -Werror): These are held back because newer Jakarta Persistence versions deprecate executeUpdate(), setMaxResults(), and getResultStream() on Query. - org.hibernate.orm:hibernate-core:7.3.4.Final - org.hibernate.orm:hibernate-hikaricp:7.3.4.Final - org.hibernate.orm:hibernate-ant:7.3.4.Final - jakarta.persistence:jakarta.persistence-api:[3.2.0,4.0.0) 2. Netty (Blocked by abandoned v5): Netty 5.0.0 was an experimental release abandoned in 2015. We explicitly cap beneath 5.0.0 so Gradle doesn't resolve dead-end alphas. - io.netty:netty-codec-http:[4.1.59.Final, 5.0.0)!! - io.netty:netty-codec:[4.1.59.Final, 5.0.0)!! - io.netty:netty-common:[4.1.59.Final, 5.0.0)!! - io.netty:netty-handler:[4.1.59.Final, 5.0.0)!! - io.netty:netty-transport:[4.1.59.Final, 5.0.0)!! - io.netty:netty-buffer:[4.1.59.Final, 5.0.0)!! 3. Google API Services: Capped beneath their respective unstable beta/v1b4 versions: - com.google.apis:google-api-services-dataflow:[v1b3-rev20240430-2.0.0, v1b4)!! - com.google.apis:google-api-services-dns:[v1-rev20240419-2.0.0, v2beta) The lockfiles have been fully regenerated and all test suites ran successfully against the latest available transitive versions.
There was a problem hiding this comment.
⚠️ Attention Required: Lockfile Detected
This pull request contains modifications to one or more *.lockfile files. Please confirm that you have run update_dependency.sh to push new dependencies to the private repo.
Someone with Admin role must manually dismiss this review before merging.
CydeWeys
left a comment
There was a problem hiding this comment.
PTAL
@CydeWeys made 1 comment.
Reviewable status: 15 of 16 files reviewed, all discussions resolved (waiting on weiminyu).
CydeWeys
left a comment
There was a problem hiding this comment.
+reviewer:@gbrodman
@CydeWeys made 1 comment.
Reviewable status: 15 of 16 files reviewed, all discussions resolved (waiting on gbrodman and weiminyu).
This commit relaxes the upper bounds on several dependencies that were previously hardcapped to specific versions:
Note: org.hibernate.orm and jakarta.persistence remain capped at 7.3.4.Final and [3.2.0,4.0.0) due to backwards incompatible deprecations added in newer versions that break the build under -Werror.
The lockfiles have been fully regenerated and all test suites ran successfully against the latest available transitive versions.
This change is