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: improve support for multi-module repos #371

Closed
dolmen opened this issue Apr 28, 2023 · 9 comments
Closed

feat: improve support for multi-module repos #371

dolmen opened this issue Apr 28, 2023 · 9 comments
Assignees
Labels
feature request New feature or request to improve the current logic

Comments

@dolmen
Copy link

dolmen commented Apr 28, 2023

Description:
Add support for caching dependencies for repositories made of multiple Go modules.

Example:

Justification:
Each Go module has its own set of dependencies. Dependencies caching should cover the union of the dependencies of all Go modules of the repo.

From the documentation, it looks like a single Go module is supported.

Are you willing to submit a PR?
No. I'm not familiar with GitHub Actions development.

@dolmen dolmen added feature request New feature or request to improve the current logic needs triage labels Apr 28, 2023
@AnatolyRugalev
Copy link

Oh wow, I am fighting exactly this right now. I think adding working-directory option will solve this issue for me

@dsame
Copy link
Contributor

dsame commented May 2, 2023

Hello @dolmen, thanks for the suggestion, we are starting to investigate it

@dsame dsame removed the needs triage label May 2, 2023
@dolmen
Copy link
Author

dolmen commented May 11, 2023

Design idea: allow to pass an array of paths for the cache-dependency-path option as an alternative to a string.

Or better: automatically process all go.sum files in the repo.

@dsame
Copy link
Contributor

dsame commented Jul 28, 2023

This feature resembles actions/setup-node#735

@dsame dsame added investigation The issue is under investigation and removed needs eyes investigation The issue is under investigation labels Aug 3, 2023
@dsame dsame self-assigned this Aug 4, 2023
@dsame
Copy link
Contributor

dsame commented Aug 8, 2023

Hello @dolmen ,

cache-dependency-path accepts both list and glob syntax:

          cache-dependency-path: |
             hello/go.sum 
             greeting/go.sum
          cache-dependency-path: "**/*.sum"

Sample workflow: https://github.com/akv-demo/test-setup-go/blob/cache-multi/.github/workflows/blank.yml
Sample build: https://github.com/akv-demo/test-setup-go/actions/runs/5799499947/job/15719631207

Did this answer help?

@dsame dsame removed the investigation The issue is under investigation label Aug 8, 2023
@dolmen
Copy link
Author

dolmen commented Aug 14, 2023

Not yet tested, but it seems to fit my need.

However, what about making **/go.sum the default?

@dolmen dolmen changed the title feat: add support for multi-module repos feat: improve support for multi-module repos Aug 14, 2023
@dsame
Copy link
Contributor

dsame commented Aug 15, 2023

@dolmen i do not think it is good idea to allow implicit dependencies. I believe the user of the action should have clear understanding of where the dependencies come from.

@dsame
Copy link
Contributor

dsame commented Aug 15, 2023

@dolmen i am going to close the issue because the problem is resolved without the implementing requested feature, but please feel free to reopen it or create new one in case if the problem still exists

@artemgavrilov
Copy link
Contributor

artemgavrilov commented Aug 25, 2023

Hi @dsame , how about make documentation regarding this a little bit more clear?
#417

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

No branches or pull requests

5 participants
@dolmen @dsame @AnatolyRugalev @artemgavrilov and others