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

Allow injecting maven settings in JDT LS sidecar #14391

Closed
l0rd opened this issue Aug 29, 2019 · 8 comments
Closed

Allow injecting maven settings in JDT LS sidecar #14391

l0rd opened this issue Aug 29, 2019 · 8 comments
Assignees
Labels
kind/enhancement A feature request - must adhere to the feature request template. severity/P1 Has a major impact to usage or development of the system.
Milestone

Comments

@l0rd
Copy link
Contributor

l0rd commented Aug 29, 2019

Is your enhancement related to a problem? Please describe.

The problem has been originally described by #14343. It's currently not possible to specify, in a devfile, maven settings that would be taken into account by the LS.

Describe the solution you'd like

We can implement a mechanism that would:

  • generate a custom settings.xml in the sidecar
  • specify the settings.xml path in the LS preferences java.configuration.maven.userSettings

That maybe done by a plugin init container (mechanism that should be introduced by #13387) that would look for MAVEN_OPTS environment variable (#14343 should be addressed first) and generate the settings.xml.

Describe alternatives you've considered

Another simpler solution that may work would be to pre-package the JDT.LS sidecar image with a settings.xml file:

  <mirrors>
    <mirror>
      <url>${env.MAVEN_MIRROR_URL}</url>
    </mirror>
  </mirrors>

Settings the environment variable MAVEN_MIRROR_URL for the ChePlugin would do the trick then (#14343 need to be fixed first).

@l0rd l0rd added kind/enhancement A feature request - must adhere to the feature request template. team/languages severity/P1 Has a major impact to usage or development of the system. labels Aug 29, 2019
@l0rd l0rd added this to the 7.2.0 milestone Aug 29, 2019
@l0rd
Copy link
Contributor Author

l0rd commented Aug 29, 2019

cc @kameshsampath

@fbricon
Copy link

fbricon commented Aug 30, 2019

indeed, starting the container with a custom settings.xml, defining a mirror (or not) and the local repository with something like <localRepository>${env.MAVEN_LOCAL_REPO}</localRepository> would work. The script starting the container needs to ensure to set the environment variables with default values (same as Maven) or bad things might happen.

cc @tsmaeder

@tsmaeder tsmaeder mentioned this issue Aug 30, 2019
41 tasks
@kameshsampath
Copy link
Contributor

kameshsampath commented Aug 30, 2019 via email

@fbricon
Copy link

fbricon commented Aug 30, 2019

if MAVEN_MIRROR_URL is not present, the settings.xml should not define the mirror

@kameshsampath
Copy link
Contributor

yeah thats what we do currently in most . of the openshift images via entrypoint scripts

@tolusha tolusha self-assigned this Sep 5, 2019
@tolusha
Copy link
Contributor

tolusha commented Sep 5, 2019

The resulted settings.xml

<settings xmlns="http://maven.apache.org/SETTINGS/1.0.0"
  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
  xsi:schemaLocation="http://maven.apache.org/SETTINGS/1.0.0
                      https://maven.apache.org/xsd/settings-1.0.0.xsd">
  <mirrors>
    <mirror>
      <url>${env.MAVEN_MIRROR_URL}</url>
      <mirrorOf>central</mirrorOf>
    </mirror>
  </mirrors>
</settings>

@tolusha
Copy link
Contributor

tolusha commented Sep 5, 2019

@fbricon I am wonder if <mirrorOf>central</mirrorOf> won't break anything.

@kameshsampath
Copy link
Contributor

kameshsampath commented Sep 5, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement A feature request - must adhere to the feature request template. severity/P1 Has a major impact to usage or development of the system.
Projects
None yet
Development

No branches or pull requests

4 participants