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

pants_supportdir causes cache thrash #3555

Closed
gmalmquist opened this Issue Jun 3, 2016 · 0 comments

Comments

Projects
None yet
2 participants
@gmalmquist
Copy link
Contributor

gmalmquist commented Jun 3, 2016

The checkstyle task has two options, properties and configuration, which may contain file paths.

In the pants repo itself, the pants.ini file contains:

[compile.checkstyle]
configuration: %(pants_supportdir)s/checkstyle/coding_style.xml 

These options are fingerprinted, and %(pants_supportdir) is expanded to an absolute path. This means that it is impossible to share cached checkstyle targets between different machines with a shared build cache.

gmalmquist added a commit to gmalmquist/pants that referenced this issue Jun 6, 2016

Make checkstyle's options filename-agnostic.
See issue: pantsbuild#3555

We were getting excessive cache invalidation in our shared build
cache for checkstyle, because it was fingerprinting the absolute
paths to the configuration files. There's really no reason for
checkstyle to care about the names of the files containing rules
and suppressions, all it should care about are the file contents.
This change enforces that.

gmalmquist added a commit to gmalmquist/pants that referenced this issue Jun 6, 2016

Make checkstyle's options filename-agnostic.
See issue: pantsbuild#3555

We were getting excessive cache invalidation in our shared build
cache for checkstyle, because it was fingerprinting the absolute
paths to the configuration files. There's really no reason for
checkstyle to care about the names of the files containing rules
and suppressions, all it should care about are the file contents.
This change enforces that.

gmalmquist added a commit to square/pants that referenced this issue Jun 6, 2016

Make checkstyle's options filename-agnostic.
See issue: pantsbuild#3555

We were getting excessive cache invalidation in our shared build
cache for checkstyle, because it was fingerprinting the absolute
paths to the configuration files. There's really no reason for
checkstyle to care about the names of the files containing rules
and suppressions, all it should care about are the file contents.
This change enforces that.

Testing Done:
Added integration tests exercising the caching behavior (unit tests don't work because FakeOptions are not fingerprinted).

CI went green here: https://travis-ci.org/pantsbuild/pants/builds/135664207

Bugs closed: 3555, 3559

Reviewed at https://rbcommons.com/s/twitter/r/3975/

gmalmquist added a commit to square/pants that referenced this issue Jun 7, 2016

Make checkstyle's options filename-agnostic.
See issue: pantsbuild#3555

We were getting excessive cache invalidation in our shared build
cache for checkstyle, because it was fingerprinting the absolute
paths to the configuration files. There's really no reason for
checkstyle to care about the names of the files containing rules
and suppressions, all it should care about are the file contents.
This change enforces that.

Testing Done:
Added integration tests exercising the caching behavior (unit tests don't work because FakeOptions are not fingerprinted).

CI went green here: https://travis-ci.org/pantsbuild/pants/builds/135664207

Bugs closed: 3555, 3559

Reviewed at https://rbcommons.com/s/twitter/r/3975/

gmalmquist added a commit to gmalmquist/pants that referenced this issue Jun 8, 2016

Make checkstyle's options filename-agnostic.
See issue: pantsbuild#3555

We were getting excessive cache invalidation in our shared build
cache for checkstyle, because it was fingerprinting the absolute
paths to the configuration files. There's really no reason for
checkstyle to care about the names of the files containing rules
and suppressions, all it should care about are the file contents.
This change enforces that.

gmalmquist added a commit to gmalmquist/pants that referenced this issue Jun 8, 2016

Make checkstyle's options filename-agnostic.
See issue: pantsbuild#3555

We were getting excessive cache invalidation in our shared build
cache for checkstyle, because it was fingerprinting the absolute
paths to the configuration files. There's really no reason for
checkstyle to care about the names of the files containing rules
and suppressions, all it should care about are the file contents.
This change enforces that.

gmalmquist added a commit to gmalmquist/pants that referenced this issue Jun 8, 2016

Make checkstyle's options filename-agnostic.
See issue: pantsbuild#3555

We were getting excessive cache invalidation in our shared build
cache for checkstyle, because it was fingerprinting the absolute
paths to the configuration files. There's really no reason for
checkstyle to care about the names of the files containing rules
and suppressions, all it should care about are the file contents.
This change enforces that.

gmalmquist added a commit to gmalmquist/pants that referenced this issue Jun 8, 2016

Make checkstyle's options filename-agnostic.
See issue: pantsbuild#3555

We were getting excessive cache invalidation in our shared build
cache for checkstyle, because it was fingerprinting the absolute
paths to the configuration files. There's really no reason for
checkstyle to care about the names of the files containing rules
and suppressions, all it should care about are the file contents.
This change enforces that.

gmalmquist added a commit to gmalmquist/pants that referenced this issue Jun 9, 2016

Make checkstyle's options filename-agnostic.
See issue: pantsbuild#3555

We were getting excessive cache invalidation in our shared build
cache for checkstyle, because it was fingerprinting the absolute
paths to the configuration files. There's really no reason for
checkstyle to care about the names of the files containing rules
and suppressions, all it should care about are the file contents.
This change enforces that.

gmalmquist added a commit to gmalmquist/pants that referenced this issue Jun 9, 2016

Make checkstyle's options filename-agnostic.
See issue: pantsbuild#3555

We were getting excessive cache invalidation in our shared build
cache for checkstyle, because it was fingerprinting the absolute
paths to the configuration files. There's really no reason for
checkstyle to care about the names of the files containing rules
and suppressions, all it should care about are the file contents.
This change enforces that.

gmalmquist added a commit to gmalmquist/pants that referenced this issue Jun 9, 2016

Make checkstyle's options filename-agnostic.
See issue: pantsbuild#3555

We were getting excessive cache invalidation in our shared build
cache for checkstyle, because it was fingerprinting the absolute
paths to the configuration files. There's really no reason for
checkstyle to care about the names of the files containing rules
and suppressions, all it should care about are the file contents.
This change enforces that.

gmalmquist added a commit to gmalmquist/pants that referenced this issue Jun 9, 2016

Make checkstyle's options filename-agnostic.
See issue: pantsbuild#3555

We were getting excessive cache invalidation in our shared build
cache for checkstyle, because it was fingerprinting the absolute
paths to the configuration files. There's really no reason for
checkstyle to care about the names of the files containing rules
and suppressions, all it should care about are the file contents.
This change enforces that.

gmalmquist added a commit to gmalmquist/pants that referenced this issue Jun 9, 2016

Make checkstyle's options filename-agnostic.
See issue: pantsbuild#3555

We were getting excessive cache invalidation in our shared build
cache for checkstyle, because it was fingerprinting the absolute
paths to the configuration files. There's really no reason for
checkstyle to care about the names of the files containing rules
and suppressions, all it should care about are the file contents.
This change enforces that.

gmalmquist added a commit to gmalmquist/pants that referenced this issue Jun 9, 2016

Make checkstyle's options filename-agnostic.
See issue: pantsbuild#3555

We were getting excessive cache invalidation in our shared build
cache for checkstyle, because it was fingerprinting the absolute
paths to the configuration files. There's really no reason for
checkstyle to care about the names of the files containing rules
and suppressions, all it should care about are the file contents.
This change enforces that.

Testing Done:
Added integration tests exercising the caching behavior (unit tests don't work because FakeOptions are not fingerprinted).

CI went green here: https://travis-ci.org/pantsbuild/pants/builds/135664207
CI went green here: http://jenkins.pantsbuild.org/job/pantsbuild/job/pants/branch/PR-3559/3/
CI went green here: https://travis-ci.org/gmalmquist/pants/builds/136502538

Bugs closed: 3555, 3559

Reviewed at https://rbcommons.com/s/twitter/r/3975/

gmalmquist added a commit to gmalmquist/pants that referenced this issue Jun 9, 2016

Make checkstyle's options filename-agnostic.
See issue: pantsbuild#3555

We were getting excessive cache invalidation in our shared build
cache for checkstyle, because it was fingerprinting the absolute
paths to the configuration files. There's really no reason for
checkstyle to care about the absolute names of the files
containing rules and suppressions, all it should care about are
the file contents. This change enforces that.

Testing Done:
Added integration tests exercising the caching behavior (unit tests don't work because FakeOptions are not fingerprinted).

CI went green here: https://travis-ci.org/pantsbuild/pants/builds/135664207
CI went green here: http://jenkins.pantsbuild.org/job/pantsbuild/job/pants/branch/PR-3559/3/
CI went green here: https://travis-ci.org/gmalmquist/pants/builds/136502538

Bugs closed: 3555, 3559

Reviewed at https://rbcommons.com/s/twitter/r/3975/

@stuhood stuhood closed this Nov 15, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment