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 using classpath jars with compiler plugins #5890

Merged
merged 3 commits into from Jun 1, 2018

Conversation

Projects
None yet
3 participants
@stuhood
Copy link
Member

stuhood commented Jun 1, 2018

Problem

In cases where --use-classpath-jars is set, the classpath entry for a target will be a ctx.jar_file rather than ctx.output_dir, but the compiler plugin map building was not accounting for this, and thus failed to inspect the (non-existent) jar file for any plugins it contained.

Solution

Refactor JvmCompile.compile and subclasses to take the CompileContext (rather than an ever-growing list of parameters), and exclude the ctx.jar_file from plugin searches.

Result

--use-classpath-jars works with compiler plugins.

@stuhood stuhood requested review from baroquebobcat , cheister and benjyw Jun 1, 2018

@stuhood

This comment has been minimized.

Copy link
Member

stuhood commented Jun 1, 2018

The actual fix for this issue is in the last commit: the first two are (minor) refactoring.

I'll look into why our existing tests didn't discover this tomorrow.

@benjyw

benjyw approved these changes Jun 1, 2018

Copy link
Contributor

benjyw left a comment

Looks great, thanks for fixing!

classpath - a list of classpath entries
outdir - the output dir to send classes to
:param vts: VersionedTargetSet with one entry for the target.
:param ctx: - A CompileContext instance for the target.

This comment has been minimized.

@benjyw

benjyw Jun 1, 2018

Contributor

Nit: Delete the leading dash.

This comment has been minimized.

@stuhood

stuhood Jun 1, 2018

Member

I'll fix this and expand the tests in a followup.

@stuhood stuhood merged commit 2ac8ef8 into pantsbuild:master Jun 1, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@stuhood stuhood deleted the twitter:stuhood/fix-use-classpath-jars-with-plugins branch Jun 1, 2018

@baroquebobcat

This comment has been minimized.

Copy link
Contributor

baroquebobcat commented Jun 1, 2018

👍

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