Added project-info flag to depmap goal

Review Request #600 — Created June 26, 2014 and submitted

ajohnson
pants
pants-reviews
ity, johanoskarsson, jsirois
Prints source roots, dependencies, and paths to libraries for targets and their dependencies in json format. 
It can be run with: ./pants goal depmap /path/to/target --depmap-project-info

Adding the -format flag causes the output to return a single line of json:
./pants goal depmap /path/to/target --depmap-project-info --depmap-format

This functionality will be used for project import in the IntelliJ Pants plugin.
Added tests for the goal (test_depmap.py), they pass

ci.sh runs successfully
  • 0
  • 0
  • 19
  • 0
  • 19
Description From Last Updated
IT
  1. A quick high level browse 
      - change the spacing to be 2 spaces (not 4) everywhere
      - use .format wherever you can
      - add in the "testing" section how you run this goal 
      - and I think you want to add "resolve" to dependencies
  2. 
      
LA
  1. What's the difference between the dependencies graph and the dependencies map (a.k.a. the thing I get from "goal depmap")?
    
    It would be good if I could figure that out from the with_descriptions of these goals.
    
    (or if the answer is "no big difference, really" then maybe this should be a flag for depmap instead of a separate goal?)
    1. There are a few differences:
      
        * this goals provides information about source roots
        * jar paths in ivy folder
        * only JVM targets are supported for now because we are going to use this goal by IntelliJ Pants plugin to generate a proper project and support auto regeneration so you don't need to regenerate it manually.
      
      Seems Alexander should change the description to something like:
      
      Print information about targets and their dependencies. Information contains source roots, dependencies and paths to libraries files.  
    2. This sounds like a superset of depmap. It'd be sweet if this was a --depmap-intellij-json flag for depmap. (Then again, maybe if I was looking at the implementations for these, maybe I'd see that was Not Easy to make that work?)
      
      For folks who are curious, I patched in this change and gave it a trial run. Pretty cool:
      
      $ PANTS_DEV=1 ./pants goal dependencies-graph tests/java/com/pants/examples/hello/greet 
      *** Running pants in dev mode from /Users/lhosken/workspace/pants/src/python/pants/bin/pants_exe.py ***
      {
          "libraries": {},
          "targets": {
              "src/java/com/pants/examples/hello/greet:greet": {
                  "libraries": [],
                  "targets": [],
                  "roots": [
                      {
                          "source_root": "/Users/lhosken/workspace/pants/src/java/com/pants/examples/hello/greet",
                          "package_prefix": "com.pants.examples.hello.greet"
                      }
                  ],
                  "test_target": false
              },
              "tests/java/com/pants/examples/hello/greet:greet": {
                  "libraries": [
                      "junit:junit-dep:4.11"
                  ],
                  "targets": [
                      "src/java/com/pants/examples/hello/greet:greet",
                      "3rdparty:junit"
                  ],
                  "roots": [
                      {
                          "source_root": "/Users/lhosken/workspace/pants/tests/java/com/pants/examples/hello/greet",
                          "package_prefix": "com.pants.examples.hello.greet"
                      }
                  ],
                  "test_target": true
              },
              "3rdparty:hamcrest-core": {
                  "libraries": [],
                  "targets": [],
                  "roots": [],
                  "test_target": false
              },
              "3rdparty:junit": {
                  "libraries": [
                      "org.hamcrest:hamcrest-core:1.3"
                  ],
                  "targets": [
                      "3rdparty:hamcrest-core"
                  ],
                  "roots": [],
                  "test_target": false
              }
          }
      }
      
      
  2. 
      
AJ
AJ
AJ
AJ
JS
  1. I haven't even opened this but from the summary/description I'm imagining we have 3 goals now that report dependencies, depmap, dependencies, and this.
    I don't think we need to block you on this, but I'd guess we all agree this is getting slightly out of hand - it would be good to have a plan for consolidation - maybe?.
    1. In the spirit of the reunion this week - we have 4 - 4 goals now that report dependencies, depmap, dependencies, resolve --ivy-open and this
    2. That's what people said when we first put this on reviewboard, so I changed it from a goal in its own right to a flag for depmap (./pants goal depmap /target/.. --depmap-project_info) 
      
      Sorry if there was any confusion, I'll go ahead and change the description to reflect the new strategy.
  2. 
      
AJ
LA
  1. Yay!
  2. += comment: if I'm thinking about changing this method's behavior, do I want to do, uhm, something to stay compatible with IntelliJ? uhm... publish a plugin? make sure AJohnson's on the review? Something?
    
    If so, good to mention that here. Because I'll forge^W^W some people might forget.
  3. 
      
FK
  1. 
      
  2. project-info
  3. change to project_info and return json object.
    
    add another option to print formatted of raw json(default to true to print formatted json).
    
    for the plugin it should be just a json line. for console testing it should be in a readable format.
  4. move to the top to other static method. might be better to call it _address_id
  5. add more tests.
    
    junit_tests and verify it's marked as test target.
    
    jvm_app and other jvm related targets
    
    also it would be wonderful to check source root if possible.
  6. 
      
AJ
FK
  1. a test for source roots will be very useful too :-)
  2. src/python/pants/backend/jvm/tasks/depmap.py (Diff revisions 2 - 3)
     
     
    no one will read this line
  3. src/python/pants/backend/jvm/tasks/depmap.py (Diff revisions 2 - 3)
     
     
     
     
     
  4. 
      
AJ
FK
  1. 
      
  2. tests/python/pants_test/tasks/test_depmap.py (Diff revisions 3 - 4)
     
     
    let's check this:
    
    'this/is/a/source/Foo.scala',
    'this/is/a/source/Bar.scala',
    
    because normaly you provide something like glob('this/is/a/source/*.scala')
  3. 
      
JO
  1. 
      
  2. This indentation looks odd
  3. 
      
AJ
IT
  1. almost there, a few nits.
  2. ?
    this is a little odd - git blame will tell us who created this. 
    1. Maybe clarify the comment something like...
      
      # Changing this task's behavior can break the Pants-IntelliJ plugin.
      # Please add fkorotkov or ajohnson to reviews for this file.
      
      ...only more accurate
  3. > 100 chars, split.
  4. >100 chars, split.
  5. defaultdict(list)?
  6. why list()?
  7. all of these that can fit in one line, please do so.
  8. #confirms -> # confirms
    
    and fix the comment.
  9. 
      
AJ
LA
  1. Yay, I can understand this code's intent. (You still want to wait for a Ship It from someone more knowledgable about how the code should work...)
  2. 
      
AJ
JS
  1. This looks good to me.  Some small stuff to clean up now and some ideas on a bigger cleanup of this monster of a Task.
  2. How about project-info-formatted, the format flag does not apply to console or graph but this is not crisply clear right now.  The description mentions JSON only in its last word.
  3. Things wen from 2 state to 3 on the output format.  It would be good to check mutual exclusivity and else die.  Then proceed to render the 1 type of output the user selected.  Right now I can say graph and json and I'll just get json.
  4. It would probably be worth circling back and converting these dict "types" into namedtuple types.  This helps define the structure seperate from the processing code and I think makes things easier to grok, especially when you want to start documenting.  I'm thinking it would be good to have a corde depmap method that produced the data model - composed of the namedtuples, and then depmap could be refactored to have 1 active ~plugin that rendered the model - be that to json or the console or what have you.  This is all forward looking at refactors to come, but it supports the idea of extracting the data model a bit and firming that up.
  5. Kill.
  6. This should do:
      return ''.join(input)
    
    That said - RB helpfully bolds builtins and its best not to shadown them, so maybe s/input/lines/
  7. 
      
AJ
JS
  1. 
      
  2. 
      
JS
  1. Thanks for your patience on this one.
    
    This is in master @ https://github.com/pantsbuild/pants/commit/c6234714ffc13b7e334b6b12efcd0eca863e3e8f
    Please mark this review as submitted.
  2. 
      
IT
  1. Ship It!
  2. 
      
AJ
Review request changed

Status: Closed (submitted)

Loading...