Fixed bugs in Go thrift generation with services

Review Request #4177 — Created Aug. 20, 2016 and submitted

mattdee123
pants
go-thrift-fixes
https://github.com/pantsbuild/pants/pull/3802
pants-reviews

Fixed bugs in Go thrift generation with services

Previously Pants would fail to work correctly for Go projects which tried to use Thrift services. This fixes two bugs related to that, and modifies a test to fail without these changes:

1) service_deps: the old way of accessing it raises an exception if service_deps is not found, but clearly None is expected when not found. The get function fixes this.

2) directory structure when symlinking files. Previously, the basename was always used as the place where the files were supposed to go. However, thrift generates a directory structure with services, and that is lost, leading to compile issues. But making source_iter contain a tuple of (source, relative_path_of_destination), we fix this issue.

Added a test that failed, confirmed that fixes make the test pass.

MA
MA
ST
  1. Looks great! Thanks for the patch, and sorry for the long delay.

    One small API fix, and then we can land this.

  2. src/python/pants/build_graph/target.py (Diff revision 1)
     
     
     
     
     
     

    Based on how you're using this, it seems like it should maybe be symmetrical to sources_relative_to_buildroot and sources_relative_to_source_root, and return the sources.

    Maybe: sources_relative_to_target_base? See self.target_base?

    1. Thanks for the response and feedback, Stu!

      I had thought of that, but opted for the previous method because this implicitly assumes that sources_relative_to_target_base and sources_relative_to_buildroot return files in the same order. They do now, so its not really a problem, just something to keep in mind.

      I've updated the patch now with your requested changes!

  3. 
      
MA
MA
Review request changed

Status: Closed (submitted)

Change Summary:

Merged as 8dd2df9915a3d05d064d11ef674b5851ed9989ca

Loading...