exclusives feature

[Brian Wickman]

The Target exclusives feature adds quite a bit of complexity to pants and I can't find any examples of it used in any of our internal repositories, so I'm curious if it's used by anybody else.  Also it seems to be a JVM-specific attribute baked directly into Target.  What's the current status of it?  Should conflicts like this be explicitly verboten instead?  Just trying to understand if this is a feature or technical debt.

[Patrick Lawson]

We have eliminated all of our exclusives groups at Foursquare and replaced the check-exclusives task (which can be quite expensive) with a noop that generates dummy products.

However I think Square might have a legitimate use case for them.  Eric?

[David Turner]

We actually do use them (in ity/science_pants), but I think doing so
might have been a mistake.

On Fri, Dec 5, 2014 at 8:25 PM, 'Brian Wickman' via Pants Developers
<pants...@googlegroups.com> wrote:

[Benjy Weinberger]

The motivating use-case for exclusives was to 

A) Prevent a target from depending on two different versions of the same 3rdparty library (e.g., thrift 0.5 and thrift 0.7). 

B) Provide a way to partition an arbitrary set of targets into disjoint "exclusives groups" - each of which depends on at most one version of any 3rdparty library. A) implies that such a partition exists.

We needed B) in order to implement "smart compile" - where we tell pants to compile everything affected by my current changes. That set might span multiple partitions, and we must compile each one separately.

It's clearly healthier not to have two different versions of the same 3rdparty library in the same repo. But I'm not sure we want to require this in every pants repo? I can imagine legitimate cases.

[Eric Zundel Ayers]

No, we aren't using exclusives and we feel that the problem that this was touted as being able to solve (conflicting resources and classes in different projects) is really only solvable by moving away from a global classpath.  We also have some projects that require java7 and others java 8 in the same repo.  Exclusives might be useful for that, but I'm not planning on using it.

[Benjy Weinberger]

Then I think we should strongly consider nuking that functionality entirely.

[Andy Reitz]

We were actually using exclusive groups in one of our monorepos for a few days last week. We had to back that change out due to some other issue, but we're hopeful that we'll be able to bring it back in use inside of Twitter. Assuming that we're able to do that, I'd vote for keeping it around.

Thanks,

  -Andy.

[John Sirois]

What does the combined Andy/David vote say?  Per Foursquare's example, we know a custom task can be setup to run early and implement exclusives more efficiently than the current setup so the argument to keep seems to me to have to be pretty well motivated at this point.

[Ity Kaul]

On Fri, Dec 5, 2014 at 10:47 PM, 'David Turner' via Pants Developers <pants...@googlegroups.com> wrote:

We actually do use them (in ity/science_pants), but I think doing so
might have been a mistake.

>> which is basically our science (one of our repos) upgrade branch to OSS pants. Didnt want folks to think that we have forked pants or something. 

On Fri, Dec 5, 2014 at 8:25 PM, 'Brian Wickman' via Pants Developers
<pants...@googlegroups.com> wrote:

> The Target exclusives feature adds quite a bit of complexity to pants and I
> can't find any examples of it used in any of our internal repositories, so
> I'm curious if it's used by anybody else.  Also it seems to be a
> JVM-specific attribute baked directly into Target.  What's the current
> status of it?  Should conflicts like this be explicitly verboten instead?
> Just trying to understand if this is a feature or technical debt.

--

Ity Kaul

Twitter Engineering

@ity

[Ity Kaul]

On Mon, Dec 8, 2014 at 11:06 AM, John Sirois <john....@gmail.com> wrote:

What does the combined Andy/David vote say?  Per Foursquare's example, we know a custom task can be setup to run early and implement exclusives more efficiently than the current setup so the argument to keep seems to me to have to be pretty well motivated at this point.

>> the amount of complexity it adds to Pants -- we are ok to remove it. We can bring it in as a custom task, if needed.

[Stu Hood]

We discussed this a bit internally, and Twitter's view on exclusives has changed since the last time this came up [0].

There is currently a very good reason to have native support for exclusives groups, and that is that the pants ideal is to support `./pants goal test ::` of your entire repo, even in a context where you have a mixed classpath ("un-hygienic", "non-isolated"?).

In the mixed classpath case without exclusives groups, pants must be deeply aware of this mixing, and must avoid batching targets in different exclusives groups together on a classpath, such as when it invokes a compiler or junit.

BUT, Twitter is interested in going all in on build hygiene, and have been in discussions with Typesafe recently about how to accomplish that [1]. Once classpaths are no longer mixed at compile or junit time, pants won't need to have a deep awareness of these restrictions (Eric mentioned this as well), at which point exclusives groups could be removed in favor of the simpler graph restrictions that Patrick mentioned Foursquare are using.

----

Given that Twitter has not had time to roll out exclusives groups at scale, and that removing exclusives groups will likely make the implementation of compile time isolation easier, we agree that it would make sense to remove them.

[0] https://groups.google.com/d/msg/pants-devel/T38IjRHurg8/D_BLUc6KYs8J

[1] design doc to follow

On Mon, Dec 8, 2014 at 11:06 AM, John Sirois <john....@gmail.com> wrote:

--

@stuhood

[Stu Hood]

If folks agree with this assessment (that hygienic build would obviate the current deep integration of exclusives in favor of shallow graph checks), I'd be interested in working on removal immediately, since I need to document the classpath manipulation portion of the codebase for future work.

--

@stuhood

[John Sirois]

On Wed, Dec 10, 2014 at 1:24 PM, 'Stu Hood' via Pants Developers <pants...@googlegroups.com> wrote:

If folks agree with this assessment (that hygienic build would obviate the current deep integration of exclusives in favor of shallow graph checks), I'd be interested in working on removal immediately, since I need to document the classpath manipulation portion of the codebase for future work.

I think we agree, but a point on terminology below.

 

On Mon, Dec 8, 2014 at 3:00 PM, Stu Hood <stu...@twitter.com> wrote:

We discussed this a bit internally, and Twitter's view on exclusives has changed since the last time this came up [0].

There is currently a very good reason to have native support for exclusives groups, and that is that the pants ideal is to support `./pants goal test ::` of your entire repo, even in a context where you have a mixed classpath ("un-hygienic", "non-isolated"?).

There is no magic here.  If pants compiles each target in isolation with its own custom resolved classpath ("hygenic build"), the error creeps back in when the targets are mixed together to form a full binary at which point a single global classpath must be picked and the conflicts will present as runtime errors!

Beyond hygenic build, pants has to support isolated runs, ie a jvm spun up per unique classpath - aka test runners - to be absolutely sure they don't induce "artificial" failures must run each test target in its own VM.  Exclusives supports this, a Task that uses the groups can so-schedule runs - I just wanted to make it very clear that "hygenic builds" is a misnomer.

[Stu Hood]

There is no magic here.  If pants compiles each target in isolation with its own custom resolved classpath ("hygenic build"), the error creeps back in when the targets are mixed together to form a full binary

Right, but that can be solved via shallow graph checks which fail the compile when X is mixed with Y, which Foursquare has already implemented.

Beyond hygenic build, pants has to support isolated runs, ie a jvm spun up per unique classpath - aka test runners - to be absolutely sure they don't induce "artificial" failures must run each test target in its own VM.

Yea, test isolation is likely more difficult, but probably 100% separable from compile isolation. It can be implemented after the compile classpath is fully isolated.

--

@stuhood

[Brian Wickman]

In any case, can we at least agree that this is a JVM specific feature and should likely live on JVM-specific targets and not 'Target' itself?

[David Turner]

Why should it be jvm-specific? Imagine two different Python targets
that rely on different versions of (say) rbtools, and we want to
ensure that they are never used together.

[John Sirois]

On Wed, Dec 10, 2014 at 3:35 PM, Brian Wickman <wic...@twitter.com> wrote:

In any case, can we at least agree that this is a JVM specific feature and should likely live on JVM-specific targets and not 'Target' itself?

Well - thats not true.  Its applicable/specific to any language with a runtime that doesn't isolate different concurrent versions of libs.  The python tester only works around this whole situation today if it runs each target in its own chroot.

[Brian Wickman]

Well in python land all of the package resolvers will spit out a VersionConflict rather than allow versions to be shadowed, but I understand your argument.

[John Sirois]

On Wed, Dec 10, 2014 at 3:44 PM, 'Brian Wickman' via Pants Developers <pants...@googlegroups.com> wrote:

Well in python land all of the package resolvers will spit out a VersionConflict rather than allow versions to be shadowed, but I understand your argument.

Ivy can be configured to blow up [1] on shadowing too - Python and Jvm langs are exactly equivalent here on both the resolution and runtime execution ends from pants point of view.  The fact that all users we know of happen to not turn on the strict or latest compatible resolvers in their ivysettings.xml is out of band and circumstantial.

[1] http://ant.apache.org/ivy/history/latest-milestone/settings/conflict-managers.html

[Stu Hood]

I've started working on removing these, but noticed that PrepareResources mutates the per-exclusives-group classpath to allow for the case of duplicated resource names. It's not obvious where resource collisions are causing the classpath to be cloned... does anyone know off the top of their heads?

There are TODOs about adding a runtime_classpath, and I'm thinking of implementing it as a merger of a compile_classpath (formerly stored in ExclusivesMapping,) and the output of PrepareResources. Sound kosher?

--

@stuhood

[Eric Zundel Ayers]

Hi Stu, 

I had my fingers in that code recently but only to make sure the ordering on the classpath matched the graph dependencies.  There are some integration tests defined under ./testprojects/maven_layout/resource_collision that make sure this is still working.

In our case, the collisions are caused by folks using multiple source_root definitions that define type 'resources' that have the same path (say, if you are using maven layout).  

It is important for us to keep the separate dir for resources per resource target and we'd like to be able to do this with other compilation output too. But I think we also concurred that this needs to be optional, as Foursquare's work says this would kill their performance.  I was hoping that having a classpath object (not just a sequence or dict) would allow us to switch out this logic easily. We could store it in products and keep separate runtime and compile variants, but I haven't thought that much about it.