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

feat: implement cache-dependency-path option to control caching dependency #499

Merged
merged 1 commit into from Nov 22, 2023

Conversation

itchyny
Copy link
Contributor

@itchyny itchyny commented Jun 10, 2023

Description:
This PR implements a new option cache-dependency-path to control caching dependency.
This option originates from setup-node, and is useful for monorepos.

Related issue:

Check list:

  • Mark if documentation changes are required.
  • Mark if tests were added or updated to cover the changes.

@itchyny itchyny requested a review from a team as a code owner June 10, 2023 04:50
@itchyny itchyny marked this pull request as draft June 10, 2023 06:17
@itchyny itchyny force-pushed the feat-cache-dependency-path branch from 81cfbd4 to 5111922 Compare June 10, 2023 07:37
@itchyny itchyny marked this pull request as ready for review June 10, 2023 07:40
@itchyny itchyny force-pushed the feat-cache-dependency-path branch from 5111922 to a56fdde Compare June 10, 2023 13:00
@itchyny
Copy link
Contributor Author

itchyny commented Jun 11, 2023

Hmm, I was worried about the complexity of listing dependency files, but I noticed that ruby/setup-ruby supports working-directory option, and that would resolve my use case. There's no actual use case to cache across multiple sub-directories. And the implementation is much simple.
https://github.com/ruby/setup-ruby/blob/v1.151.0/index.js#L45
actions/setup-node#706

@itchyny
Copy link
Contributor Author

itchyny commented Jun 24, 2023

Any chance to get this merged? Despite my previous comment, I think this is the actions/* way since this option is supported not only by setup-node, but setup-go and setup-python.

@IvanZosimov
Copy link
Contributor

Hi, @itchyny 👋 Thank you for the contribution! We will take a look and get back to you with our decision regarding it.

@itchyny
Copy link
Contributor Author

itchyny commented Jun 28, 2023

Oops, I thought the CI failure is something flaky, but it's just about code formatting. I'll fix this soon.

@itchyny itchyny force-pushed the feat-cache-dependency-path branch from a56fdde to 50b7edd Compare June 28, 2023 14:29
@IvanZosimov IvanZosimov added feature request New feature or request to improve the current logic needs eyes labels Jul 7, 2023
@itchyny itchyny force-pushed the feat-cache-dependency-path branch from 50b7edd to a8e0c55 Compare July 24, 2023 23:19
src/cache.ts Outdated
return `${CACHE_KEY_PREFIX}-${process.env['RUNNER_OS']}-${packageManager.id}-${hash}`;
async function computeCacheKey(
packageManager: PackageManager,
cacheDependencyPath?: string
Copy link
Contributor

Choose a reason for hiding this comment

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

In the current implementation of the getInput() function, cacheDependencyPath can't be undefined as if it's not supplied to the workflow file, its value will be an empty string.

src/cache.ts Show resolved Hide resolved
@IvanZosimov
Copy link
Contributor

Hi @itchyny, I left some comments to your PR, please check them out. Also, I think it'd be great to update the list of inputs in the action.yml file and add some e2e tests for the new functionality. Examples of e2e tests you can find in the ./.github/workflows/e2e-cache.yml file.

@IvanZosimov IvanZosimov self-assigned this Nov 8, 2023
@itchyny itchyny marked this pull request as draft November 9, 2023 22:03
@itchyny itchyny force-pushed the feat-cache-dependency-path branch 6 times, most recently from 8d6fd83 to 3a33295 Compare November 10, 2023 11:27
@itchyny itchyny marked this pull request as ready for review November 10, 2023 11:28
src/cache.ts Show resolved Hide resolved
src/cache.ts Show resolved Hide resolved
src/cache.ts Show resolved Hide resolved
@IvanZosimov IvanZosimov merged commit 9eda6b5 into actions:main Nov 22, 2023
259 checks passed
zeebe-bors-camunda bot added a commit to camunda/zeebe that referenced this pull request Dec 7, 2023
15484: deps(github-tags): Update actions/setup-java action to v4 (main) r=oleschoenburg a=renovate[bot]

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [actions/setup-java](https://togithub.com/actions/setup-java) | action | major | `v3.13.0` -> `v4.0.0` |
| [actions/setup-java](https://togithub.com/actions/setup-java) | action | major | `v3` -> `v4` |

---

> [!WARNING]
> Some dependencies could not be looked up. Check the Dependency Dashboard for more information.

---

### Release Notes

<details>
<summary>actions/setup-java (actions/setup-java)</summary>

### [`v4.0.0`](https://togithub.com/actions/setup-java/releases/tag/v4.0.0)

[Compare Source](https://togithub.com/actions/setup-java/compare/v3.13.0...v4.0.0)

#### What's Changed

In the scope of this release, the version of the Node.js runtime was updated to 20. The majority of dependencies were updated to the latest versions. From now on, the code for the setup-java will run on Node.js 20 instead of Node.js 16.

#### Breaking changes

-   Update Node.js runtime to version 20 by [`@&#8203;aparnajyothi-y](https://togithub.com/aparnajyothi-y)` in [actions/setup-java#558

#### Non-breaking changes

-   Adding support for microsoft openjdk 21.0.0 by [`@&#8203;ralfstuckert](https://togithub.com/ralfstuckert)` in [actions/setup-java#546
-   Update [`@&#8203;actions/cache](https://togithub.com/actions/cache)` dependency and documentation by [`@&#8203;IvanZosimov](https://togithub.com/IvanZosimov)` in [actions/setup-java#549
-   Implementation of the cache-dependency-path option to control caching dependency by [`@&#8203;itchyny](https://togithub.com/itchyny)` in [actions/setup-java#499

#### New Contributors

-   [`@&#8203;ralfstuckert](https://togithub.com/ralfstuckert)` made their first contribution in [actions/setup-java#546
-   [`@&#8203;itchyny](https://togithub.com/itchyny)` made their first contribution in [actions/setup-java#499

**Full Changelog**: actions/setup-java@v3...v4.0.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 8pm every weekday,before 6am every weekday" (UTC), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these updates again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/camunda/zeebe).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy44MS4zIiwidXBkYXRlZEluVmVyIjoiMzcuODcuMiIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->


Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request to improve the current logic
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants