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

Provide input parameters for key environment variable supported by GitHub Dependency Graph plugin #193

Open
bigdaz opened this issue Apr 20, 2024 · 5 comments
Labels
enhancement New feature or request good first issue Good for newcomers in:dependency-submission

Comments

@bigdaz
Copy link
Member

bigdaz commented Apr 20, 2024

Currently, there are some key configuration inputs to the GitHub Dependency Graph plugin that can only be provided via environment variables.
The full list is here, here and here.

We don't need to support all of these variables, since some are set by the action and there's little or no reason to override them. We also only need to support these for dependency-submission. When setup-gradle with dependency-graph enabled, users can continue to provide environment variables.

For now, we should at least provide:

  • report-dir: DEPENDENCY_GRAPH_REPORT_DIR with paths resolved relative to the project workspace
  • exclude-projects: DEPENDENCY_GRAPH_EXCLUDE_PROJECTS
  • exclude-configurations: DEPENDENCY_GRAPH_EXCLUDE_CONFIGURATIONS
  • Possibly the 'include' variations as well.

For now we can leave out the variables that control the 'runtime' scope, since these are largely experimental. Ideally, the plugin would make a good guess at the dependencies that are in runtime scope, but it's not trivial.

@radoslaw-panuszewski
Copy link

When setup-gradle with dependency-graph enabled, users can continue to provide environment variables.

For the setup-gradle users, the configuration via environment variables is very inconvenient right now.

We first tried the naive approach:

steps:
  - uses: gradle/actions/setup-gradle@v3
    env:
      DEPENDENCY_GRAPH_REPORT_DIR: ${{ runner.temp }}/setup-gradle 

  - run: ./gradlew build

But it failed with a strange Groovy error:

Initialization script '/home/github/.gradle/init.d/gradle-actions.github-dependency-graph.init.gradle' line: 43

* What went wrong:
A problem occurred evaluating initialization script.
> Ambiguous method overloading for method java.io.File#<init>.
  Cannot resolve which method to invoke for [null, class java.lang.String] due to overlapping prototypes between:
  	[class java.lang.String, class java.lang.String]
  	[class java.io.File, class java.lang.String]

After a little of debugging, it turned out that the DEPENDENCY_GRAPH_REPORT_DIR is not exported globally and the step executing ./gradlew doesn't see it. The issue is that maybeExportVariable doesn't export variable if it's already present:

function maybeExportVariable(variableName: string, value: unknown): void {
    if (!process.env[variableName]) {
        core.exportVariable(variableName, value)
    }
}

To make it working, we need to globally export the variable. Currently, we do it this way:

steps:
  - run: echo "DEPENDENCY_GRAPH_REPORT_DIR=${{ runner.temp }}/setup-gradle" >> $GITHUB_ENV
    shell: bash

  - uses: gradle/actions/setup-gradle@v3

  - run: ./gradlew build

As I said, it's inconvenient for us. We would love to have the report-dir input parameter for setup-gradle and I will happily provide a PR if I get a green light for that ;) @bigdaz

@bigdaz
Copy link
Member Author

bigdaz commented Apr 29, 2024

I would prefer to restrict these input parameters to the dependency-submission action if possible. Although it works, I'm not (yet) sure if there's a good use case for having dependency-submission incorporated into the regular build process, as opposed to having it done in a dedicated Job/workflow.

Here are some options you have right now:

  • If possible, use the dependency-submission action but specify the dependency-resolution-task to run.
    • You can then provide the DEPENDENCY_GRAPH_REPORT_DIR variable to the action step.
  • Configure the DEPENDENCY_GRAPH_REPORT_DIR environment variable on the workflow or Job, rather than on a particular step.

@bigdaz
Copy link
Member Author

bigdaz commented Apr 29, 2024

Ah, I now see that with the fixed support for a custom report directory, the env var needs to be visible to both the setup-gradle step AND the steps that execute the Gradle build(s). So it makes sense that we export the existing variable value. The maybeExportVariable function was designed to avoid overwriting any existing setting with the defaults.

We could change this function to always export the variable, using the existing value if it is set and using the default value if not.

A PR to fix this would be welcome.

@radoslaw-panuszewski
Copy link

If possible, use the dependency-submission action but specify the dependency-resolution-task to run.
You can then provide the DEPENDENCY_GRAPH_REPORT_DIR variable to the action step.

In our case, it's a company-wide migration to use the setup-gradle action in our workflows (around 1800 repositories to change). We considered usage of the dependency-submission action but we rejected it because we see additional benefits of using the setup-gradle, for example better caching. We have our internal wrapper for the setup-gradle to share common defaults, like enabling the dependency graph generation. Having this wrapper also gives us the opportunity to provide other common Gradle configs, like access to our Artifactory (this way we don't have to spread the credentials throughout all the repositories).

So basically, we say to our developers: "use our allegro/setup-gradle action and we will configure all Gradle stuff which is needed in our organization". We prefer to hide the dependency graph generation from the devs, because they don't care about it. We, as an infrastructure team, are the only ones interested in having the SBOMs submitted to Github.

Configure the DEPENDENCY_GRAPH_REPORT_DIR environment variable on the workflow or Job, rather than on a particular step.

We can't do that, because we use the gradle/actions/setup-gradle in a composite action, not directly in the workflow.

I can provide a PR which changes maybeExportVariable to always export the variable, but I also find our use case sensible enough to have the report-dir input parameter for setup-gradle. Both options will work for me, although I find the second one cleaner 😉

Please let me know what you think 🙂

@bigdaz
Copy link
Member Author

bigdaz commented Apr 29, 2024

As I stated, I'd prefer not to add more dependency-submission specific input parameters to setup-gradle.

I think a PR to change maybeExportVariable would be useful regardless, since the current behaviour is confusing. Right now if we use the default value, this will be available for all subsequent steps. But if a user sets the value explicitly for the setup-gradle action, then this value will not be available for subsequent steps (where it is required).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers in:dependency-submission
Projects
None yet
Development

No branches or pull requests

2 participants