Design strategy for externalizing Target fields #3560

stuhood · 2016-06-06T21:24:38Z

One of the largest end-user-visible semantic changes in the engine experiments was the idea of moving from typed Target classes to untyped Targets holding typed "products". And the most beneficial part of that change was moving from typed Targets to targets holding a typed Sources subclass as a product.

This ticket deals with designing a strategy to begin exposing typed Sources from targets, which should begin to erode the need for typed Target subclasses. It blocks #2996, which hopes to move graph-mutation (including codegen/deferred-sources) tasks into the new engine.

In terms of scope, this issue should likely reach far enough down the road to have a reasonable answer to how we might migrate from java_library(.., sources=..) to target(.., java(globs=..)), and whether that would require a major version bump to 2.x.

cc @jsirois, @patricklaw , @benjyw

The text was updated successfully, but these errors were encountered:

In cases where a TargetMacro has been installed as wrapping an existing symbol, it will not exist in the `target_aliases` field, and will thus not be replaced with our adaptor. "Why yes, this continues to highlight the importance of:" #3560 #3561 Testing Done: https://travis-ci.org/pantsbuild/pants/builds/140649055 And tested on internal TargetMacro usecases. Bugs closed: 3506, 3607 Reviewed at https://rbcommons.com/s/twitter/r/4000/

1.1.0-rc0 (7/1/2016) -------------------- This is the first `1.1.0-rc` release on the way to the `1.1.0`. New Features ~~~~~~~~~~~~ * Subprocess clean-all `RB pantsbuild#4011 <https://rbcommons.com/s/twitter/r/4011>`_ * expose products for jvm bundle create and python binary create tasks `RB pantsbuild#3959 <https://rbcommons.com/s/twitter/r/3959>`_ `RB pantsbuild#4015 <https://rbcommons.com/s/twitter/r/4015>`_ * Implement zinc `unused deps` check `RB pantsbuild#3635 <https://rbcommons.com/s/twitter/r/3635>`_ API Changes ~~~~~~~~~~~ * Add `is_target_root` in export `RB pantsbuild#4030 <https://rbcommons.com/s/twitter/r/4030>`_ Bugfixes ~~~~~~~~ * ConsoleRunner bugfix for @TestSerial and other test cleanups `RB pantsbuild#4026 <https://rbcommons.com/s/twitter/r/4026>`_ New Engine Work ~~~~~~~~~~~~~~~ * [engine] Proper implementation of `**` globs in the v2 engine `RB pantsbuild#4034 <https://rbcommons.com/s/twitter/r/4034>`_ * [engine] Fix TargetMacro replacements of adapted aliases `Issue pantsbuild#3560 <https://github.com/pantsbuild/pants/issues/3560>`_ `Issue pantsbuild#3561 <https://github.com/pantsbuild/pants/issues/3561>`_ `RB pantsbuild#4000 <https://rbcommons.com/s/twitter/r/4000>`_ Refactoring, Improvements, and Tooling ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ * Fix dead apidocs link for guava. `RB pantsbuild#4037 <https://rbcommons.com/s/twitter/r/4037>`_ * Bump setproctitle to 1.1.10. `Issue pantsbuild#44 <https://github.com/dvarrazzo/py-setproctitle/issues/44>`_ `RB pantsbuild#4035 <https://rbcommons.com/s/twitter/r/4035>`_ * Set a default read timeout for fetching node pre-installed modules. 1 second default often fails `RB pantsbuild#4025 <https://rbcommons.com/s/twitter/r/4025>`_ * Improve stderr handling for ProcessManager.get_subprocess_output(). `RB pantsbuild#4019 <https://rbcommons.com/s/twitter/r/4019>`_ * Add AnnotatedParallelClassesAndMethodsTest* and AnnotatedParallelMethodsTest* `RB pantsbuild#4027 <https://rbcommons.com/s/twitter/r/4027>`_

1.1.0-rc0 (7/1/2016) -------------------- This is the first `1.1.0-rc` release on the way to the `1.1.0`. New Features ~~~~~~~~~~~~ * Subprocess clean-all `RB pantsbuild#4011 <https://rbcommons.com/s/twitter/r/4011>`_ * expose products for jvm bundle create and python binary create tasks `RB pantsbuild#3959 <https://rbcommons.com/s/twitter/r/3959>`_ `RB pantsbuild#4015 <https://rbcommons.com/s/twitter/r/4015>`_ * Implement zinc `unused deps` check `RB pantsbuild#3635 <https://rbcommons.com/s/twitter/r/3635>`_ API Changes ~~~~~~~~~~~ * Add `is_target_root` in export `RB pantsbuild#4030 <https://rbcommons.com/s/twitter/r/4030>`_ Bugfixes ~~~~~~~~ * ConsoleRunner bugfix for @TestSerial and other test cleanups `RB pantsbuild#4026 <https://rbcommons.com/s/twitter/r/4026>`_ New Engine Work ~~~~~~~~~~~~~~~ * [engine] Proper implementation of `**` globs in the v2 engine `RB pantsbuild#4034 <https://rbcommons.com/s/twitter/r/4034>`_ * [engine] Fix TargetMacro replacements of adapted aliases `Issue pantsbuild#3560 <https://github.com/pantsbuild/pants/issues/3560>`_ `Issue pantsbuild#3561 <https://github.com/pantsbuild/pants/issues/3561>`_ `RB pantsbuild#4000 <https://rbcommons.com/s/twitter/r/4000>`_ Refactoring, Improvements, and Tooling ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ * Fix dead apidocs link for guava. `RB pantsbuild#4037 <https://rbcommons.com/s/twitter/r/4037>`_ * Bump setproctitle to 1.1.10. `Issue pantsbuild#44 <https://github.com/dvarrazzo/py-setproctitle/issues/44>`_ `RB pantsbuild#4035 <https://rbcommons.com/s/twitter/r/4035>`_ * Set a default read timeout for fetching node pre-installed modules. 1 second default often fails `RB pantsbuild#4025 <https://rbcommons.com/s/twitter/r/4025>`_ * Improve stderr handling for ProcessManager.get_subprocess_output(). `RB pantsbuild#4019 <https://rbcommons.com/s/twitter/r/4019>`_ * Add AnnotatedParallelClassesAndMethodsTest* and AnnotatedParallelMethodsTest* `RB pantsbuild#4027 <https://rbcommons.com/s/twitter/r/4027>`_ Testing Done: CI pending: https://travis-ci.org/pantsbuild/pants/builds/141717353 Bugs closed: 3620 Reviewed at https://rbcommons.com/s/twitter/r/4042/

baroquebobcat · 2016-08-11T20:19:10Z

This might also allow not requesting sources for tasks that don't use them. With the current implementation, list for example, will evaluate all the source globs.

stuhood · 2017-01-27T20:07:05Z

Getting rid of Target inheritance entirely is not going to happen anytime soon.

But one strategy that allows us to move more pre-computation into the engine is to begin externalizing target arguments, so that the engine can compute more before passing them in... and then, eventually, not pass them in.

I think my strategy will be to define a series of Field instances as class properties of the relevant Target subclass, which the engine can use while hydrating a target to pre-hydrate its fields. This code will likely need to replace Payload (eventually).

### Problem In order to compute source files signature, v2 engine looks for `sources` field, now with 5813a2d , it needs to support implicit sources with `sources` field is not present. ### Solution The approach in the rb is more short term/cheap to continue to use `TargetAdaptor`, extends this hierarchy to have default source suffixes and excludes there. The alternative/longer term solution is #3560. ### Result See the new test in `test_graph.test_implicit_sources`, w/o the change it would fail: ``` except Exception as e: raise TargetDefinitionException( target_adaptor.address, > 'Failed to instantiate Target with type {}: {}'.format(target_cls, e)) E TargetDefinitionException: Invalid target BuildFileAddress(testprojects/tests/python/pants/file_sets/BUILD, implicit_sources): Failed to instantiate Target with type <class 'pants.backend.python.targets.python_library.PythonLibrary'>: Unrecognized command line flags on global scope: --color, --confcutdir, --resultlog .pants.d/python-setup/chroots/a447b84a0a5e9517301fc0570a067595fbecdf2f/pants/engine/legacy/graph.py:158: TargetDefinitionException ```

stuhood · 2017-03-16T18:22:59Z

Retitled to more accurately cover the current goal.

stuhood · 2017-04-04T22:54:21Z

Relevant to this task: we need to make sure the new design integrates tightly with the --changed options: see #4420 (comment)

stuhood · 2017-04-10T18:27:11Z

Bit of a stretch, but I'm targetting this to 1.3.0 so that we can have as much thought as possible put into this before the 1.3.0 milestone, in case there are any obvious deprecations.

### Problem In order to compute source files signature, v2 engine looks for `sources` field, now with 5813a2d , it needs to support implicit sources with `sources` field is not present. ### Solution The approach in the rb is more short term/cheap to continue to use `TargetAdaptor`, extends this hierarchy to have default source suffixes and excludes there. The alternative/longer term solution is pantsbuild#3560. ### Result See the new test in `test_graph.test_implicit_sources`, w/o the change it would fail: ``` except Exception as e: raise TargetDefinitionException( target_adaptor.address, > 'Failed to instantiate Target with type {}: {}'.format(target_cls, e)) E TargetDefinitionException: Invalid target BuildFileAddress(testprojects/tests/python/pants/file_sets/BUILD, implicit_sources): Failed to instantiate Target with type <class 'pants.backend.python.targets.python_library.PythonLibrary'>: Unrecognized command line flags on global scope: --color, --confcutdir, --resultlog .pants.d/python-setup/chroots/a447b84a0a5e9517301fc0570a067595fbecdf2f/pants/engine/legacy/graph.py:158: TargetDefinitionException ```

stuhood · 2017-04-25T19:15:40Z

Deprioritized #4439 in order to work on this a bit instead, because I believe it's important to have pieces of this and all of #4508 figured out before cutting 1.3.0.

stuhood · 2017-04-27T00:46:52Z

Drafted an initial summary of some concepts in the area, and our goals. Will work on actually proposing what to do to externalize fields tomorrow.

https://docs.google.com/document/d/102EFbwk6cpM9-_4ZSYhMQYA0zL1UKbXzW-DCd8KsFeg/edit?usp=sharing

stuhood · 2017-04-27T22:04:05Z

Tiny bit more progress on documenting history, but didn't actually dive into the proposed design... too busy today. Tomorrow!

stuhood · 2017-04-28T21:26:52Z

This is now reviewable here: https://docs.google.com/document/d/102EFbwk6cpM9-_4ZSYhMQYA0zL1UKbXzW-DCd8KsFeg/edit?usp=sharing cc @jsirois , @kwlzn , @baroquebobcat , @benjyw

jsirois · 2017-05-01T16:55:42Z

LGTM, niggly comment left that's really addressable in code review. The design of schema factory method, instance values method makes good sense to transition.

stuhood · 2017-05-01T18:19:06Z

Ok... I think this is a plan. Closing this... will follow up in #4535

stuhood added the engine label Jun 6, 2016

This was referenced Jun 6, 2016

Python legacy resources are not included in filedeps or changed #3563

Closed

TargetDefinitionException when using globs with resources= in a PythonLibrary #3506

Closed

stuhood added the engine-bootstrap label Jun 14, 2016

stuhood mentioned this issue Jun 14, 2016

Implement deferred sources using ivy in the new engine #3336

Closed

stuhood removed the engine-codegen label Aug 8, 2016

stuhood mentioned this issue Oct 31, 2016

Python resource_targets args not accounted for in changed #4010

Closed

stuhood mentioned this issue Nov 7, 2016

Support composing named "bundle"/"sourceset" targets into a JvmApp #2068

Closed

stuhood mentioned this issue Jan 27, 2017

No sources fingerprinted in v2 in the presence of implicit_sources #4213

Closed

stuhood self-assigned this Jan 27, 2017

stuhood mentioned this issue Feb 17, 2017

make Target cacheable in the ProductGraph #3991

Closed

peiyuwang mentioned this issue Feb 28, 2017

[engine] Support implicit sources in v2 engine #4294

Merged

stuhood changed the title ~~Design strategy for moving to Sources-as-a-Product~~ Design strategy for externalizing Target fields Mar 16, 2017

stuhood mentioned this issue Apr 4, 2017

[engine] v2 changed doesn't find linkages to non-dependencies= declared deps. #4420

Closed

stuhood added this to the 1.3.0 milestone Apr 10, 2017

stuhood mentioned this issue Apr 10, 2017

Products as objects; replace string representation of Products with objects #628

Closed

stuhood removed their assignment Apr 12, 2017

stuhood mentioned this issue Apr 24, 2017

Replace traversable_*_specs #4508

Closed

stuhood self-assigned this Apr 25, 2017

stuhood closed this as completed May 1, 2017

stuhood mentioned this issue Jun 7, 2017

Broken SourcesField support for globs #4641

Closed

stuhood mentioned this issue Jun 19, 2018

Deprecate legacy default sources API #5987

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Design strategy for externalizing Target fields #3560

Design strategy for externalizing Target fields #3560

stuhood commented Jun 6, 2016 •

edited

Loading

baroquebobcat commented Aug 11, 2016

stuhood commented Jan 27, 2017

stuhood commented Mar 16, 2017

stuhood commented Apr 4, 2017

stuhood commented Apr 10, 2017

stuhood commented Apr 25, 2017

stuhood commented Apr 27, 2017

stuhood commented Apr 27, 2017

stuhood commented Apr 28, 2017

jsirois commented May 1, 2017 •

edited

Loading

stuhood commented May 1, 2017

Design strategy for externalizing Target fields #3560

Design strategy for externalizing Target fields #3560

Comments

stuhood commented Jun 6, 2016 • edited Loading

baroquebobcat commented Aug 11, 2016

stuhood commented Jan 27, 2017

stuhood commented Mar 16, 2017

stuhood commented Apr 4, 2017

stuhood commented Apr 10, 2017

stuhood commented Apr 25, 2017

stuhood commented Apr 27, 2017

stuhood commented Apr 27, 2017

stuhood commented Apr 28, 2017

jsirois commented May 1, 2017 • edited Loading

stuhood commented May 1, 2017

stuhood commented Jun 6, 2016 •

edited

Loading

jsirois commented May 1, 2017 •

edited

Loading