pantsbuild #general

Channels

welcome

github-notifications

announce

jolly-chef-92794

03/18/2016, 9:47 PM

So every task has to have a custom fingerprint strategy to take that into account?

ancient-stone-50795

03/18/2016, 9:47 PM

@jolly-chef-92794: you don't have to include every field in a target in the fingerprint. we just always leave that one out is what I think stu is saying

witty-crayon-22786

03/18/2016, 9:47 PM

@jolly-chef-92794: it could be blacklist based

witty-crayon-22786

03/18/2016, 9:47 PM

yea

witty-crayon-22786

03/18/2016, 9:48 PM

...although i guess that would mean you'd need a "default" jvm strategy?

witty-crayon-22786

03/18/2016, 9:48 PM

not sure how else you can drop fields.

jolly-chef-92794

03/18/2016, 9:48 PM

But then things blow up if you change a

compile

scope to `runtime`; jvm_compile will hit the cache and assume it still works

witty-crayon-22786

03/18/2016, 9:49 PM

@jolly-chef-92794: it should inspect that scope if it matters for compilation...?

ancient-stone-50795

03/18/2016, 9:49 PM

no, I don't think so. the computed classpath for targets that depend on it will be different.

jolly-chef-92794

03/18/2016, 9:50 PM

So in my patch I added this test case, which didn’t pass without me adding the scope information into the target fingerprint https://github.com/gmalmquist/pants/blob/gmalmquist/provided-dependencies/tests/python/pants_test/backend/jvm/tasks/test_scope_runtime_integration.py#L119

witty-crayon-22786

03/18/2016, 9:53 PM

well, you either need it in the fingerprint or in the strategy

ancient-stone-50795

03/18/2016, 9:53 PM

Did you update the fingerprint code that jvm_compile is using to use your new scoped_closure() and friends methods?

witty-crayon-22786

03/18/2016, 9:53 PM

that much is clear?

ancient-stone-50795

03/18/2016, 9:53 PM

great minds think alike

ancient-stone-50795

03/18/2016, 9:54 PM

coke

witty-crayon-22786

03/18/2016, 9:54 PM

heh

jolly-chef-92794

03/18/2016, 9:55 PM

So I was correct in saying that"So every task has to have a custom fingerprint strategy to take that into account?” earlier, if it’s not in the target?

ancient-stone-50795

03/18/2016, 9:56 PM

I don't know how custom it needs to be, but each class should take into account whether it is using the runtime classpath or the compile classpath to be sure.

ancient-stone-50795

03/18/2016, 9:56 PM

If that wasn't done before, it seems like it was an oversight.

jolly-chef-92794

03/18/2016, 9:57 PM

Well I mean it was by using the scoped_closure

jolly-chef-92794

03/18/2016, 9:57 PM

when computing the classpath

jolly-chef-92794

03/18/2016, 9:57 PM

But it doesn’t even get as far as computing the classpath if it thinks the targets haven’t changed

jolly-chef-92794

03/18/2016, 9:58 PM

In any event, I’m looking into rewriting the patch with scope on the targets now

ancient-stone-50795

03/18/2016, 10:01 PM

Release PR is out for real now: https://rbcommons.com/s/twitter/r/3586/

bored-art-40741

03/18/2016, 10:11 PM

⛵

witty-crayon-22786

03/18/2016, 10:18 PM

@jolly-chef-92794: thanks. note that it might be a bit more than that... transitivity on a dependent might make sense, but scope probably doesn't

jolly-chef-92794

03/18/2016, 10:21 PM

@witty-crayon-22786: So like a

intransitive_dependencies=[…]

field?

witty-crayon-22786

03/18/2016, 10:22 PM

possibly

witty-crayon-22786

03/18/2016, 10:26 PM

"But it doesn’t even get as far as computing the classpath if it thinks the targets haven’t changed" <- not true in the case of jvm_compile

ancient-stone-50795

03/19/2016, 5:12 PM

@bored-art-40741: Could you go into https://pypi.python.org/pypi/pantsbuild.pants.contrib.android and add me and other releasers as owners of this pakcage? I can't get past first base in the release script until that is done.