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

Replace actions/checkout with actions/upload/download #3602

Merged
merged 3 commits into from
Oct 23, 2019

Conversation

alpeb
Copy link
Member

@alpeb alpeb commented Oct 18, 2019

So far just a test, please don't review

@alpeb alpeb self-assigned this Oct 18, 2019
@alpeb alpeb force-pushed the alpeb/replace-actionscheckout-with-upload/download branch from d10c3c2 to d323155 Compare October 18, 2019 21:54
@alpeb
Copy link
Member Author

alpeb commented Oct 18, 2019

@siggy this is ready for review. I've removed all the action/checkout entries except for the one at the top. It ended up in quite a lot of repetitions, but I don't know if there's anything we can do about it. Let me know what you think.

@alpeb alpeb requested a review from siggy October 18, 2019 22:03
Copy link
Member

@siggy siggy left a comment

Choose a reason for hiding this comment

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

awesome to see this!

i wouldn't worry too much about the repetition. we already have a lot of that. eventually we could move the repeated bits into their own actions, or wait for yaml anchor support.

the current test failure looks related to a dirty repo.

browsing through a couple stack overflow questions, it sounds like copying a git repo is not a great idea. instead, we could just tar and upload the .git directory, download it to a temp location (i think {{ runner.temp }} or $RUNNER_TEMP will work?), and then git clone {{ runner.temp }} {{ github.workspace }}

.github/workflows/workflow.yml Outdated Show resolved Hide resolved
.github/workflows/workflow.yml Outdated Show resolved Hide resolved
.github/workflows/workflow.yml Outdated Show resolved Hide resolved
.github/workflows/workflow.yml Outdated Show resolved Hide resolved
@alpeb
Copy link
Member Author

alpeb commented Oct 21, 2019

@siggy this is ready for review again. The tests were green!

Copy link
Member

@siggy siggy left a comment

Choose a reason for hiding this comment

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

Very excited to see this, great work! ✅ ✅ ✅

Signed-off-by: Alejandro Pedraza Borrero <[email protected]>
@alpeb
Copy link
Member Author

alpeb commented Oct 23, 2019

Notes for reviewers:

This replaces the multiple calls to actions/checkout as a workaround to actions/checkout#27
With this PR, there's a new download_src job at the beginning that calls actions/checkout, tars the .git directory and then uploads it into the builds' artifact storage using actions/upload-artifact.

Ever other job depends on download_src. Now instead of calling actions/checkout again, they use actions/download-artifact to download the tarball, untar it into the runner's temporary storage dir, and clone from it into the workspace.

Copy link
Contributor

@ihcsim ihcsim left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this 👍.

@alpeb alpeb merged commit 397970e into master Oct 23, 2019
@alpeb alpeb deleted the alpeb/replace-actionscheckout-with-upload/download branch October 23, 2019 22:23
alpeb added a commit that referenced this pull request Jan 7, 2020
alpeb added a commit that referenced this pull request Jan 7, 2020
alpeb added a commit that referenced this pull request Jan 8, 2020
* Revert "Replace actions/checkout with actions/upload/download (#3602)"

This reverts commit 397970e.

* Upgraded actions/checkout to @v2

Reverts #3602 and Fixes #3881
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants