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

Fix path sensitivity of compile tasks #1568

Merged
merged 2 commits into from
Nov 26, 2020

Conversation

iamdanfox
Copy link
Contributor

@iamdanfox iamdanfox commented Nov 26, 2020

Before this PR

Our build caching is currently kinda broken, because you can't get cache hits between local & CI. build scan link

image

Specifically, on a local run the java compiler would be invoked with the following argument, which contains an absolute path 🌶🌶🌶:

-XepDisableWarningsInGeneratedCode -XepExcludedPaths:\Q/Volumes/git/container-vuln-scanner/container-vuln-scanner-api\E\Q/\E(build|src\Q/\Egenerated.*)\Q/\E.* -Xep:FallThrough -Xep:UnnecessaryParentheses -Xep:CatchSpecificity:OFF -Xep:UnusedVariable:OFF -Xep:EqualsHashCode:ERROR -Xep:EqualsIncompatibleType:ERROR -Xep:StreamResourceLeak:ERROR -Xep:InputStreamSlowMultibyteRead:ERROR -Xep:JavaDurationGetSecondsGetNano:ERROR -Xep:URLEqualsHashCode:ERROR -Xep:BoxedPrimitiveEquality:ERROR -Xep:ReferenceEquality:ERROR -XepOpt:LogsafeArgName:UnsafeArgNames=branch,branchid,branchname,branchrid,email,emailaddress,organization,organizationname,path,ref,secret,transactionref,token,username

When the repo is the checked out to a different location (e.g. /home/circleci/container-vuln-scanner), it computes a different set of args and causes a cache miss.

cc @jmcampanini this is example number (3) of gradle enterprise saving the day

After this PR

==COMMIT_MSG==
JavaCompile tasks should now get more build cache hits irrespective of the location of your repo on disk, as baseline-errorprone no longer injects an absolute path into errorproneOptions.excludedPaths.
==COMMIT_MSG==

Interestingly I think that windows compilation was also broken, and that this might fix it.

Possible downsides?

@changelog-app
Copy link

changelog-app bot commented Nov 26, 2020

Generate changelog in changelog/@unreleased

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

JavaCompile tasks should now get more build cache hits irrespective of the location of your repo on disk, as baseline-errorprone no longer injects an absolute path into errorproneOptions.excludedPaths.

Check the box to generate changelog(s)

  • Generate changelog entry

Copy link
Contributor

@fawind fawind left a comment

Choose a reason for hiding this comment

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

Good catch!

@bulldozer-bot bulldozer-bot bot merged commit 252ed9c into develop Nov 26, 2020
@bulldozer-bot bulldozer-bot bot deleted the dfox/fix-compilation-cache branch November 26, 2020 10:22
@svc-autorelease
Copy link
Collaborator

Released 3.57.1

errorProneOptions
.getExcludedPaths()
.set(String.format(
"%s%s(build|src%sgenerated.*)%s.*",
Pattern.quote(projectPath), separator, separator, separator));
".*(build%sgenerated%ssources|src%sgenerated.*)%s.*",
Copy link
Contributor

Choose a reason for hiding this comment

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

This does not seem to handle cases that previously were excluded such as build/metricSchema/generated_src used by metric-schema which is causing the baseline bump excavator on tritium to fail compilation: https://app.circleci.com/pipelines/github/palantir/tritium/906/workflows/7c65b97b-b475-46fc-a1e1-b5a4bf7198a0/jobs/11565

Filed #1570

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.

4 participants