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

[SPARK-32695][INFRA] Explicitly cache and hash 'build' directly in GitHub Actions #29536

Closed
wants to merge 7 commits into from

Conversation

HyukjinKwon
Copy link
Member

@HyukjinKwon HyukjinKwon commented Aug 25, 2020

What changes were proposed in this pull request?

This PR proposes to explicitly cache and hash the files/directories under 'build' for SBT and Zinc at GitHub Actions. Otherwise, it can end up with overwriting build directory. See also #29286 (comment)

Previously, other files like build/mvn and build/sbt are also cached and overwritten. So, when you have some changes there, they are ignored.

Why are the changes needed?

To make GitHub Actions build stable.

Does this PR introduce any user-facing change?

No, dev-only.

How was this patch tested?

The builds in this PR test it out.

@SparkQA

This comment has been minimized.

This reverts commit cfe5e92.
@SparkQA

This comment has been minimized.

@SparkQA

This comment has been minimized.

@SparkQA

This comment has been minimized.

@HyukjinKwon HyukjinKwon changed the title [SPARK-32695][INFRA] Add 'build' and 'project/build.properties' into cache key of SBT and Zinc [SPARK-32695][INFRA] Explicitly cache and hash 'build' directly in GitHub Actions Aug 25, 2020
@HyukjinKwon
Copy link
Member Author

cc @dongjoon-hyun and @gengliangwang can you take a look when you're available?

@SparkQA
Copy link

SparkQA commented Aug 25, 2020

Test build #127872 has finished for PR 29536 at commit 42d4e5c.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@@ -99,10 +99,14 @@ jobs:
run: git merge --progress --ff-only origin/${{ github.event.inputs.target }}
# Cache local repositories. Note that GitHub Actions cache has a 2G limit.
- name: Cache Scala, SBT, Maven and Zinc
uses: actions/cache@v1
uses: actions/cache@v2
Copy link
Member

Choose a reason for hiding this comment

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

why is the difference between v1 and v2?

Copy link
Member Author

Choose a reason for hiding this comment

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

In order to allow multiple file path (https://github.com/actions/cache#whats-new). It is also to make it consistent in this file. cache@v2 is used in all places at this file except here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Previously, other files like build/mvn and build/sbt are also cached and overwritten. So, when you have some changes there, they are ignored.

build/zinc-*
build/scala-*
build/*.jar
key: build-${{ hashFiles('**/pom.xml', 'project/build.properties', 'build/mvn', 'build/sbt', 'build/sbt-launch-lib.bash', 'build/spark-build-info') }}
Copy link
Member

Choose a reason for hiding this comment

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

(Ah, I see. glob patterns like build/*' cannot be used in hashFilesnow...)

@maropu
Copy link
Member

maropu commented Aug 25, 2020

Looked around the GH actions docs and this update looks resonable to me.

Copy link
Member

@gengliangwang gengliangwang left a comment

Choose a reason for hiding this comment

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

LGTM

@HyukjinKwon
Copy link
Member Author

Thanks @maropu and @gengliangwang!

@HyukjinKwon
Copy link
Member Author

Merged to master. branch-3.0 and branch-2.4.

HyukjinKwon added a commit that referenced this pull request Aug 26, 2020
…tHub Actions

### What changes were proposed in this pull request?

This PR proposes to explicitly cache and hash the files/directories under 'build' for SBT and Zinc at GitHub Actions. Otherwise, it can end up with overwriting `build` directory. See also #29286 (comment)

Previously, other files like `build/mvn` and `build/sbt` are also cached and overwritten. So, when you have some changes there, they are ignored.

### Why are the changes needed?

To make GitHub Actions build stable.

### Does this PR introduce _any_ user-facing change?

No, dev-only.

### How was this patch tested?

The builds in this PR test it out.

Closes #29536 from HyukjinKwon/SPARK-32695.

Authored-by: HyukjinKwon <gurwls223@apache.org>
Signed-off-by: HyukjinKwon <gurwls223@apache.org>
(cherry picked from commit b07e742)
Signed-off-by: HyukjinKwon <gurwls223@apache.org>
HyukjinKwon added a commit that referenced this pull request Aug 26, 2020
…tHub Actions

### What changes were proposed in this pull request?

This PR proposes to explicitly cache and hash the files/directories under 'build' for SBT and Zinc at GitHub Actions. Otherwise, it can end up with overwriting `build` directory. See also #29286 (comment)

Previously, other files like `build/mvn` and `build/sbt` are also cached and overwritten. So, when you have some changes there, they are ignored.

### Why are the changes needed?

To make GitHub Actions build stable.

### Does this PR introduce _any_ user-facing change?

No, dev-only.

### How was this patch tested?

The builds in this PR test it out.

Closes #29536 from HyukjinKwon/SPARK-32695.

Authored-by: HyukjinKwon <gurwls223@apache.org>
Signed-off-by: HyukjinKwon <gurwls223@apache.org>
(cherry picked from commit b07e742)
Signed-off-by: HyukjinKwon <gurwls223@apache.org>
@HyukjinKwon HyukjinKwon deleted the SPARK-32695 branch December 7, 2020 02:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants