Add ParseContext singleton helper #4466

Merged
merged 7 commits into from Apr 14, 2017

Conversation

Projects
None yet
2 participants
@stuhood
Member

stuhood commented Apr 14, 2017

Problem

Because no capability for parse-local state existed, IntermediateTargetFactory used class-local state to determine whether a singleton intermediate target had been created, rather than parse-local state.

The result was fragile... a subtle symptom (#4416) was that after a buildfile was invalidated in the daemon, the singleton targets it contained would not be recreated, causing them to go missing completely. A more obvious symptom was that init.py needed to reset the class-level state to ensure that intermediate targets could be recreated.

Solution

Add support for parse-level state via a new method create_object_if_not_exists on ParseContext, and mark the already effectively-public methods of ParseContext as such.

As an orthogonal change, move from using a lock to guard the collection of created objects to using a thread-local collection.

Result

Daemon runs that involve intermediate/'scoped'/'provided' targets that have been invalidated will succeed. Fixes #4416

@stuhood stuhood requested review from jsirois, gmalmquist and JieGhost Apr 14, 2017

@JieGhost

I think it's pretty neat.
Thanks for removing the ugly hack in IntermediateTargetFactoryBase!

@stuhood

This comment has been minimized.

Show comment
Hide comment
@stuhood

stuhood Apr 14, 2017

Member

I see further cleanup possible here, but the public API will work... shipping it.

Member

stuhood commented Apr 14, 2017

I see further cleanup possible here, but the public API will work... shipping it.

@stuhood stuhood merged commit 9c9ef2a into pantsbuild:master Apr 14, 2017

1 check passed

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

@stuhood stuhood deleted the twitter:stuhood/parse-context-create_object_if_not_exists branch Apr 14, 2017

lenucksi added a commit to lenucksi/pants that referenced this pull request Apr 25, 2017

Add ParseContext singleton helper (#4466)
### Problem

Because no capability for parse-local state existed, `IntermediateTargetFactory` used class-local state to determine whether a singleton intermediate target had been created, rather than parse-local state.

The result was fragile... a subtle symptom (#4416) was that after a buildfile was invalidated in the daemon, the singleton targets it contained would not be recreated, causing them to go missing completely. A more obvious symptom was that `init.py` needed to reset the class-level state to ensure that intermediate targets could be recreated.

### Solution

Add support for parse-level state via a new method `create_object_if_not_exists` on `ParseContext`, and mark the already effectively-public methods of `ParseContext` as such.

As an orthogonal change, move from using a lock to guard the collection of created objects to using a thread-local collection.

### Result

Daemon runs that involve intermediate/'scoped'/'provided' targets that have been invalidated will succeed. Fixes #4416
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment