Honoring configured `networkTimeout` when calling `./gradle wrapper` by gmazzo · Pull Request #14043 · dependabot/dependabot-core

gmazzo · 2026-01-29T10:02:13Z

What are you trying to accomplish?

Fixes #14027 by workaround gradle/gradle#36172, by reading networkTimeout property from gradle-wrapper.properties and passing it as --network-timeout when calling ./gradlew wrapper.

Anything you want to highlight for special attention from reviewers?

As I commented at #14024 (comment), I don't think we should be patching Gradle's defects. But in case you decide to move forward, this should be a way to do it.

How will you know you've accomplished your goal?

Tests and smoke tests will cover it

Checklist

I have run the complete test suite to ensure all tests and linters pass.
I have thoroughly tested my code changes to ensure they work as expected, including adding additional tests for new functionality.
I have written clear and descriptive commit messages.
I have provided a detailed description of the changes in the pull request, including the problem it addresses, how it fixes the problem, and any relevant details about the implementation.
I have ensured that the code is well-documented and easy to understand.

yeikel · 2026-01-29T13:59:13Z

-        sig { params(properties_file: T.any(Pathname, String)).returns(T.nilable(String)) }
-        def get_validate_distribution_url_option(properties_file)
-          return nil unless File.exist?(properties_file)
+        # For the same reason, we apply the same for `--network-timeout`.


Let's add the right context here please. It is not exactly the same reason.

Hopefully we can merge #14037 first, or you can cherry-pick my commit and we merge it as a single change; It does not matter to me as long as the context is captured

If we follow the pattern in #14037, we can add it as we define the argument rather that in the method docs

Just solved conflicts and added some wording for it. Feel free to make it better ;)

yeikel · 2026-01-29T14:02:15Z

+        # For the same reason, we apply the same for `--network-timeout`.
+        sig { params(properties_file: T.any(Pathname, String)).returns(T::Array[T.nilable(String)]) }
+        def read_wrapper_options(properties_file)
+          return [nil, nil] unless File.exist?(properties_file)


I think that this check should not be here, instead, it should be before this method is called

If there is no property file all the way to this context, the subsequent logic should not run at all

To be fair, based on what I currently see in the code, I don't even think that's possible either

I'd prefer to keep it in here to be honest. If, for whatever reason, the file is not there, then there are no values to read, so it's the same as if they are not present at all in the .properties file. It keeps the logic encapsulated.

In what situation can the property file not exist and get to this context? Maybe I am missing something

The reason I ask is because extra error handling does not help if it can never happen. It does not hurt in terms of error handling, it is just dead code

But maybe I am missing something

yeikel · 2026-01-29T14:06:01Z

              env = { "JAVA_OPTS" => proxy_args.join(" ") } # set proxy for gradle execution

+              command_parts = %w(--no-daemon --stacktrace) + command_args(target_requirements, network_timeout)
+              command = Shellwords.join([has_local_script ? "./gradlew" : "gradle"] + command_parts)


Can we remove this workaround in this change or in a future change?

I am honestly not too sure this can happen. From what I remember, this was added because of fears that users may not commit the Unix variant, but honestly, I am not too sure if we should actually even do this. Of all the hacks, this is the one I regret the most because Dependabot works in Unix and we can set that as an expectation

No, this was because:

It could be possible that ./gradlew script was altered, not working (without +x permission) or even not existing at all, and that will dependabot to fail.

Or the gradle-wrapper.jar is corrupted, compiled for an incompatible java version, et.

So, panic condition, if updating with the current wrapper helper code fails, we'll try with system's native gradle instalation before giving up.

Actually, one of the smoke tests is validating this scenario.

Sorry, I meant the has_local_script check. I understand the fallback mechanism, what I don't understand is when has_local_script can be false

We don't know the structure of a project. Is possible they do have a gradle/gradle-wrapper.properties file, but for some reason (for instance, they only support Windows) they have decided not to commit the ./gradlew script.

That's the thing, we don't know. This is more to have a robust code prepared for any kind of scenario (that we can predict, without entering too much complexity).

Thanks, I think that we should log this behavior in a follow up change. Just for user's awareness.

yeikel · 2026-01-29T14:07:57Z

This change closes #14026

yeikel · 2026-01-30T14:52:49Z

Changes look good to me. Thanks for the effort

@kbukum1 Please review when you are ready. Also see the discussion as to why I believe that we should still do this even if it feels that Gradle should do it

yeikel · 2026-02-02T02:16:13Z

          its(:content) do
            expected_command = %W(
              ./gradlew --no-daemon --stacktrace wrapper --gradle-version 9.0.0 --no-validate-url
+              --network-timeout 10000


As this is the default value, can we please create a test to ensure that other than the default is respected? Maybe an integration test?

kbukum1 · 2026-02-02T20:56:41Z

Changes look good to me. Thanks for the effort

@kbukum1 Please review when you are ready. Also see the discussion as to why I believe that we should still do this even if it feels that Gradle should do it

@gmazzo , thanks for the effort. I did review it roughly. I will review again later. Currently there are issues on actions and pipeline is failing. I will review later and most probably approve since I see you guys did great job went on every detail. @yeikel thank you for all reviews and questions. It helps a lot.

Co-authored-by: Yeikel Santana <email@yeikel.com>

yeikel · 2026-02-04T05:32:36Z

The build failure seems like a temporary downtime. Retrying the build should fix it

github-actions Bot added the L: java:gradle Maven packages via Gradle label Jan 29, 2026

This was referenced Jan 29, 2026

CI PR, do not merge gmazzo/dependabot-core#6

Closed

Do not change networkTimeout value in gradle-wrapper.properties #14024

Closed

gmazzo force-pushed the gradle-wrapper-network-timeout-fix branch 2 times, most recently from 9553f04 to 6fa2d2f Compare January 29, 2026 11:01

yeikel reviewed Jan 29, 2026

View reviewed changes

Comment thread gradle/lib/dependabot/gradle/file_updater/wrapper_updater.rb Outdated

gmazzo force-pushed the gradle-wrapper-network-timeout-fix branch 4 times, most recently from 17f0f51 to 3d826d2 Compare January 30, 2026 09:06

yeikel reviewed Jan 30, 2026

View reviewed changes

Comment thread gradle/lib/dependabot/gradle/file_updater/wrapper_updater.rb Outdated

gmazzo marked this pull request as ready for review February 1, 2026 19:39

gmazzo requested a review from a team as a code owner February 1, 2026 19:39

yeikel reviewed Feb 2, 2026

View reviewed changes

kbukum1 self-assigned this Feb 2, 2026

kbukum1 moved this to On Hold in Dependabot Feb 2, 2026

kbukum1 added this to Dependabot Feb 2, 2026

kbukum1 force-pushed the gradle-wrapper-network-timeout-fix branch from c47aa13 to 15dabae Compare February 4, 2026 03:16

kbukum1 approved these changes Feb 4, 2026

View reviewed changes

kbukum1 moved this from On Hold to In Progress in Dependabot Feb 4, 2026

kbukum1 force-pushed the gradle-wrapper-network-timeout-fix branch from 15dabae to 2a4d4d1 Compare February 4, 2026 03:49

gmazzo and others added 2 commits February 3, 2026 20:02

Honoring configured networkTimeout when calling ./gradle wrapper

b801f23

Update gradle/lib/dependabot/gradle/file_updater/wrapper_updater.rb

2dd6bf3

Co-authored-by: Yeikel Santana <email@yeikel.com>

JamieMagee force-pushed the gradle-wrapper-network-timeout-fix branch from 2a4d4d1 to 2dd6bf3 Compare February 4, 2026 04:02

Merge branch 'main' into gradle-wrapper-network-timeout-fix

dec47a9

kbukum1 merged commit cd91cb5 into dependabot:main Feb 4, 2026
93 of 94 checks passed

github-project-automation Bot moved this from In Progress to Done in Dependabot Feb 4, 2026

Conversation

gmazzo commented Jan 29, 2026 • edited by kbukum1 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What are you trying to accomplish?

Anything you want to highlight for special attention from reviewers?

How will you know you've accomplished your goal?

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yeikel Jan 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yeikel Jan 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yeikel commented Jan 29, 2026

Uh oh!

Uh oh!

Uh oh!

yeikel commented Jan 30, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kbukum1 commented Feb 2, 2026

Uh oh!

yeikel commented Feb 4, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gmazzo commented Jan 29, 2026 •

edited by kbukum1

Loading

yeikel Jan 29, 2026 •

edited

Loading

yeikel Jan 29, 2026 •

edited

Loading