A `remote_sources` target as a better mechanism for from_target.

Review Request #4014 — Created June 20, 2016 and submitted

gmalmquist
pants
gmalmquist/a-better-fix-for-deferred-sources
3592
pants-reviews
jsirois, stuhood, zundel

Stu suggested in https://rbcommons.com/s/twitter/r/3830/ that we
overhaul the deferred sources system to generate synthetic targets
instead. This should eliminate a lot of special casing we've been
having to do around deferred sources, since we can use the same
logic that was already handling synthetic targets created from
generated code.

This change adds the `remote_sources` target as a replacement for
the `from_target` object, and deprecates from_target.

In the meantime, I added an option to `deferred_sources_mapper`
to throw an exception if `from_target` is used, which will allow
individual users to eliminate their own usage of `from_target` in
favor of the more robust `remote_sources`.

Added unit test.

Jenkins went green here: http://jenkins.pantsbuild.org/job/pantsbuild/job/pants/job/PR-3592/2/
CI went green here: https://travis-ci.org/pantsbuild/pants/builds/139023132
Jenkins went green here: http://jenkins.pantsbuild.org/job/pantsbuild/job/pants/job/PR-3592/4/

  • 0
  • 0
  • 1
  • 0
  • 1
Description From Last Updated
  1. 
      
  2. Can you provide some pydoc for this (or expand on the class pydoc above) so it will get published to the website and people will know how to use it?

  3. I don't see any tests where you exercised this parameter.

  4. What is the purpose of this? To get ready to deprecate from_target()? If so, then add to help text.

  5. 
      
  1. Ship It!
  2. 
      
  1. Looks great! Would like to see the old solution deprecated immediately though. Lot of weird hacks surrounding it that will be good to remove asap.

  2. I would prefer to see this be deprecated now, so that we can start the clock on removal and drop it in 1.3.0.

    Also, it seems like from_target can be deprecated as well?

  3. 
      
  1. Ship It!
  2. 
      
  1. So I just looked at the general scheme and did not do a deep read of the implementation. We don't use this feature internally. But I strongly agree with the choice to change to using synthetic targets here and this all looks good to me.

    I unpack_libraries for the android backend and this is akin to what I did there - unpack the libs and inject a dependency instead of changing the fingerprint of the dependent target.

  2. 
      
Review request changed

Status: Closed (submitted)

Change Summary:

In 25f5b99c9249f572a744ae0a26ab00bc6f43e58c; thanks Eric, Stu, and Mateo.

Loading...