scalameta / metals-vscode

Visual Studio Code extension for Metals
https://marketplace.visualstudio.com/items?itemName=scalameta.metals#overview
Apache License 2.0
299 stars 76 forks source link

Missing HTTP proxy settings for Coursier when fetching Metals #1497

Closed mliarakos closed 4 months ago

mliarakos commented 4 months ago

Describe the bug

Since v1.30.0, Coursier fails to fetch Metals when an HTTP proxy is needed.

To Reproduce

Steps to reproduce the behavior:

  1. Install v1.30.0 of the extension
  2. Configure the serverProperties setting with the HTTP proxy, e.g.:
    • -Dhttps.proxyHost=...
    • -Dhttps.proxyPort=...
  3. Trigger the Metals extension install process
  4. It fails to fetch Metals with an unknown host exception (due to the missing proxy configuration)

This happens both if Coursier is already present on the PATH and if fetched by the extension.

Expected behavior

During the install process Coursier uses the HTTP proxy configuration from the serverProperties setting to successfully fetch Metals.

Screenshots

None.

Installation:

Additional context

In v1.29.0 and earlier the fetchMetals method includes the serverProperties settings as JVM arguments when invoking the default included Coursier. However, in v1.30.0 and newer no external settings are included in the Coursier call to fetch Metals.

I believe the same issue occurs when trying to specify an HTTP proxy using $JAVA_OPTS or the .jvmopts file. Those options are included in v1.29.0, but not in v1.30.0. This is just based on the code, I wasn't using it that way so I didn't test it.

This issue might also be present in v1.29.0 when using an external copy of Couriser, but I didn't test that either.

Search terms

coursier, proxy

tgodzik commented 4 months ago

Thanks for reporting, looks like another issue we need to fix, that should be fairly easy I think. Will take a look!

LeUser111 commented 4 months ago

I've been encountering this issue with multiple colleagues since version 1.30.0 and with version 1.34.0 the issue is back.

Here are my user settings:

"metals.mavenScript": "/usr/bin/mvn",
"metals.sbtScript": "/opt/sbt/bin/sbt",
"metals.javaHome": "/opt/jdk",
"metals.scalafmtConfigPath": "/home/user/.scalafmt.conf",
"metals.scalafixConfigPath": "/home/user/.scalafix.conf",
"metals.ammoniteJvmProperties": [
    "-Dhttps.proxyHost=127.0.0.1",
    "-Dhttps.proxyPort=3128"
],
"metals.bloopJvmProperties": [
    "-Dhttps.proxyHost=127.0.0.1",
    "-Dhttps.proxyPort=3128"
],
"metals.serverProperties": [
    "-Xmx2G",
    "-Dhttps.proxyHost=127.0.0.1",
    "-Dhttps.proxyPort=3128",
    "-Djavax.net.ssl.trustStore=/home/user/.m2/grobCaTrustStore.jks",
    "-Djavax.net.ssl.trustStorePassword=changeit"
],
"metals.customRepositories": [
    "https://nexuss/nexus/repository/public/"
],
"metals.coursierMirror": "https://nexuss/nexus/repository/public/",

I have removed any additional environment variables (e.g. JAVAOPTS) and specific settings (e.g. ~/.bloop/bloop.json and ~/.config/coursier/mirror.properties) except ~/.config/coursier/credentials.properties_

I've just been running multiple tests removing ~/.cache/coursier/ each time. Things are working with version 1.29.0, 1.32.0 and 1.33.0 (see attached files). Versions 1.30.0, 1.31.0 and 1.34.0 ignore the server settings and the installation fails (see attached files).

There are two anomalies with the working versions, as well: Bloop fails to start initially, requiring an explicit 'Import build'. That leads to more dependencies being downloaded. That works, as the build server starts afterwards and metals is fully working. However, the download of the bloop dependencies gets reported as ERROR.

One more oddity: I have to specify -Djavax.net.ssl.trustStore for coursier even though the CA certificate has been added to /opt/jdk and any other build tool (maven and sbt) uses the certificte from there...

metals_log_1.29.0.txt metals_log_1.30.0.txt metals_log_1.31.0.txt metals_log_1.32.0.txt metals_log_1_33_0.txt metals_log_1.34_0.txt

Edit: Grammar.

mliarakos commented 4 months ago

@LeUser111 The fix in #1498 was released in v1.32.0 and reverted in v1.34.0, which explains the changes you're seeing.

@tgodzik Why did it need to be reverted? I can submit a modified PR if needed.

tgodzik commented 4 months ago

The extension on windows stopped working at all so I had to revert some of the changes for now. I plan to get back to it as soon as I can

tgodzik commented 4 months ago

Looks like we can't add the properties on windows in coursier. Adding back the changes aside from that in https://github.com/scalameta/metals-vscode/pull/1510

I will double check if it works and then will release new version

tgodzik commented 4 months ago

This should be now fixed, any chance anyone can confirm? You would need to switch to a prerelease version.

I will also want to check if everything works correctly on windows before doing the release.

LeUser111 commented 4 months ago

@tgodzik The pre-release version is working :) Thank you for the quick fix and thank you for your work on metals in general! It's coming along really nicely :)

@mliarakos Thanks for the initial fix!

tgodzik commented 4 months ago

Thanks for checking! I released an official version, there still might be some issue on Windows though.

LeUser111 commented 4 months ago

The release version 1.35.0 is working fine on several machines for us :) Good luck with Windows!