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

Gradle: Support for proxy detection / autoconfiguration. #5006

Merged
merged 1 commit into from
Dec 7, 2022

Conversation

sdedic
Copy link
Member

@sdedic sdedic commented Nov 25, 2022

On a corporate network or on a VPN the user often forgets about a network change - even worse because with PAC autoconfiguration, NetBeans IDE is able to dynamically change proxy / non-proxy, depending on PAC results.

But build tools like Gradle have to get explicit instructions. The user uses NetBeans, happilly browses online resources (with an PAC-configured browser), but build actions or even project info load itself fails mysteriously on missing on unreachable artifacts.

This PR adds a check, before taking an 'online' action in Gradle, if Gradle's configuration file matches IDE's own proxy settings. If it does not, the default is to suggest the user to reconfigure Gradle - and NetBeans will even do that for the user.
proxy2

The user may also choose to leave settings as they are (for whatever reason), but let the IDE to instruct Gradle to use proxy for IDE-initiated operations. In this scenario, commandline actions will likely fail - but no persistent change in files need to be made.

A global Gradle setting is introduced to specify the preferred handling:

  • ask the user (the default)
  • do not check
  • display a notice and override
  • override using system properties
  • automatically update

Old gradle.properties are always saved as gradle.properties.old (gradle.properties.old.1, ....) to preserve potential user content, although EditableProperties API is used to retain eventual user comments in the file.

The current implementation contains some cut-through path, since because of gradle/gradle#22856 I am able to define a proxy, if one is not defined at all, but the bug prevents me from changing or removing proxy settings on-the-fly in case the proxy is configured in the file, but the config is wrong.

Note: because of Vscode handling, the option to pass system properties ("Override" option) can be disabled by branding API.

@sdedic sdedic added enhancement Gradle [ci] enable "build tools" tests labels Nov 25, 2022
@sdedic sdedic added this to the NB17 milestone Nov 25, 2022
@sdedic sdedic self-assigned this Nov 25, 2022
@lkishalmi
Copy link
Contributor

Would not be this better as a project problem provider with a resolution?

@sdedic
Copy link
Member Author

sdedic commented Nov 25, 2022

I would advise against that, but it is surely doable. In siuations where the DNS does not reject the proxy name immediately (timeout) or when the intranet simply drops (not rejects) outgoing packets, gradle app + network timeouts are huuuuuuge. It may take several to dozen minutes for the project load to fail.
It seemed safer + better user experience to hook before any non-offline operation and check in advance.

@sdedic
Copy link
Member Author

sdedic commented Nov 29, 2022

I would appreciate if this PR could land in NB16u1-gradle (cc: @MartinBalin) -- in NB a misconfigured proxy is bad enough, but if the vscode environment is used with several lang servers all working against bad proxy/network configuration, it is a true hell on developer's machine.

@neilcsmith-net
Copy link
Member

neilcsmith-net commented Nov 29, 2022

@sdedic you'll need to open a second PR, cherry picking these changes on top of release160, taking care of module versions if necessary (I assume 2.29.1 but check).

@MartinBalin I'll liaise with you when we can start merging into the release branch - not until the existing votes are closed anyway!

@sdedic
Copy link
Member Author

sdedic commented Nov 29, 2022

@neilcsmith-net understood - but I'd like to get the code in here reviewed or sanity-checked first

Copy link
Contributor

@dbalek dbalek left a comment

Choose a reason for hiding this comment

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

Useful functionality, Thanks.

@sdedic
Copy link
Member Author

sdedic commented Dec 6, 2022

@lkishalmi -- last call: there's sanity check from @dbalek, but this is your field of expertise, I'd feel more comfortable if I could get +-0 from you for this implementation.

@sdedic sdedic added the 16u1 label Dec 6, 2022
Copy link
Contributor

@lkishalmi lkishalmi left a comment

Choose a reason for hiding this comment

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

I like the idea, that this feature flagged!

@sdedic sdedic merged commit bd38774 into apache:master Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Gradle [ci] enable "build tools" tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants