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

Java: Please either autodetect the needed JDK version from pom.xml or document that setup-java must be called manually #2074

Open
wtwhite opened this issue Jan 9, 2024 · 2 comments

Comments

@wtwhite
Copy link

wtwhite commented Jan 9, 2024

For CodeQL runs on Maven Java projects, the default-generated workflow, which runs autobuild, fails with error: invalid target release: 17 if the project requires JDK 17 (or, apparently, any version besides JDK 8). The same failure occurs if autobuild is replaced with a basic mvn compile manual build step.

Several issues have comments describing a solution (namely, adding a setup-java action) -- but I could not find any mention of it in the CodeQL config docs (1, 2, 3, 4). In fact, these docs strongly suggest that setting up the right version of Java build tools is only required if using self-hosted runners:

  • From CodeQL code scanning for compiled languages:

    Note: If you use self-hosted runners for GitHub Actions, you may need to install additional software to use the autobuild process. Additionally, if your repository requires a specific version of a build tool, you may need to install it manually. GitHub-hosted runners are always run with the software required by autobuild.

  • Further down:

    If you're using self-hosted runners, the required version(s) of Java should be present:

    If the runner will be used for analyzing repositories that need a single version of Java, then the appropriate JDK version needs to be installed, and needs to be present in the PATH variable (so that java and javac can be found).

    If the runner will be used for analyzing repositories that need multiple versions of Java, then the appropriate JDK versions need to be installed, and can be specified via the toolchains.xml file. This is a configuration file, typically used by Apache Maven, that allows you to specify the location of the tools, the version of the tools, and any additional configuration that is required to use the tools. For more information, see "Guide to Using Toolchains" in the Apache Maven documentation.

Many people have tripped over this: 1, 2, 3, 4, 5.

There was already an intention to autodetect the needed JDK version, which would be the ideal solution. (It could be implemented by looking for any <maven.compiler.target/> element in pom.xml.) But if that's tricky, then please at least document that manually adding setup-java to the workflow is needed for JDK versions other than 8.

@aibaars
Copy link
Collaborator

aibaars commented Jan 9, 2024

GitHub-hosted runners are always run with the software required by autobuild.

That statement is overly optimistic ;-) GitHub-hosted runners have several versions of Java pre-installed, but it definitively does not contain every possible version. According to internal.ubuntu-22.04.json the current github-hosted runner image contains:

        {
          "NodeType": "HeaderNode",
          "Title": "Java",
          "Children": {
            "NodeType": "TableNode",
            "Headers": "Version|Environment Variable",
            "Rows": [
              "8.0.392+8|JAVA_HOME_8_X64",
              "11.0.21+9 (default)|JAVA_HOME_11_X64",
              "17.0.9+9|JAVA_HOME_17_X64",
              "21.0.1+12|JAVA_HOME_21_X64"
            ]
          }
        },

The CodeQL autobuilder looks in the pom.xml file for hints about the required Java version and should pick the best matching version from the ones above and otherwise falls back to JAVA_HOME or the one on the PATH. The actions/setup-java Action sets JAVA_HOME to a user specified Java version, and is indeed the right fix in case CodeQL cannot determine the right version to use or when the right version is not available on the runner.

Could you share the pom.xml file and relevant bits of the logs for investigation? Perhaps the autobuilder's version guessing logic can be improved.

@smowton
Copy link
Contributor

smowton commented Jan 9, 2024

NB. we have intended to detect needed version from pom.xml more ambitiously for a while, but that work is stuck because we found that roughly equal numbers of repositories were fixed thanks to finding useful cues to install a particular version, as were deceived into installing an incompatible Java version by some misleading apparent hint in the pom.xml file. We intend to revisit the matter to try to refine our heuristics, but it's not easy.

jo-pol added a commit to DANS-KNAW-jp/dd-ingest-flow that referenced this issue Jan 12, 2024
jo-pol added a commit to DANS-KNAW-jp/dd-ingest-flow that referenced this issue Jan 12, 2024
jjjasper added a commit to Backbase/backbase-openapi-tools that referenced this issue Mar 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants