Improve detect-duplicates to detect dups in internal dependencies

Review Request #420 — Created May 28, 2014 and submitted

ity
pants
pants-reviews
jsirois, patricklaw
Improve detect-duplicates to detect duplicate classes and resources in internal and external dependencies
Retake of https://rbcommons.com/s/twitter/r/394/

I have removed the test classes I added as they were conflicting with real dependencies (having the same package name)
And this time ci was run after a clean.
$ ./build-support/bin/ci.sh
Build operating on top level addresses: set([BuildFileAddress(tests/python/pants_test/BUILD, integration)])
============================================================================= test session starts ==============================================================================
platform darwin -- Python 2.6.8 -- py-1.4.20 -- pytest-2.5.2
plugins: cov, timeout
collected 2 items

tests/python/pants_test/tasks/test_jar_publish_integration.py ..

========================================================================== 2 passed in 20.70 seconds ===========================================================================
tests.python.pants_test.tasks.jar_publish_integration                           .....   SUCCESS

[== CI SUCCESS ==]
  • 1
  • 0
  • 0
  • 0
  • 1
Description From Last Updated
The excludes are still a single comma-sep string, so although unlikely for self._excludes = 'a,b,c' you'd incorrectly exclude a file ... JS jsirois
PA
  1. 
      
  2. zip is a builtin, I'd recommend choosing a different variable name.
  3. Closing does this for you
  4. 
      
IT
PA
  1. I can't comment too much on the semantics of how the detection works, but the code looks good AFAICT.
  2. 
      
IT
  1. Thanks Patrick - in terms of semantics of the checks - it basically tries to find any common qualified classnames or resources (such as com.a.A in the dependencies of a project X and warns during the binary goal) 
    
    The output looks something like this (I had this on the previous RB as an example):
    
    $ ./pants goal binary src/java/com/twitter/common/application:test-application -ldebug
    
    18:06:14 00:00 [main]
                   (To run a reporting server: ./pants goal server)
    18:06:14 00:00   [bootstrap]
    18:06:14 00:00   [setup]
    18:06:14 00:00     [parse]
                   Preparing goals in round 1
    .......
                        ===== For target JvmBinary(BuildFileAddress(src/java/com/twitter/common/application/BUILD, test-application)):
                       Duplicate classes and/or resources detected in artifacts: (u'com.twitter.common-application-0.0.72.jar', u'src.java.com.twitter.common.application.test-application.jar')
                            com/twitter/common/application/Application.class
                   SUCCESS
    
    
    Hope that helps! Take 2 in trying to get this in! 
  2. 
      
JS
  1. The one marked issue is an unlikely to be triggered bug that should be fixed before submitting.
  2. The excludes are still a single comma-sep string, so although unlikely for self._excludes = 'a,b,c' you'd incorrectly exclude a file named 'a,b'.  It would be good to turn this into a list, better a frozenset, of strings early in the constructor.
  3. prefer ("..."
            "...")
    
    The style is split, but the latest / only public back-forth had Benjy & Wickman discuss this on a review and no \ won.
  4. 
      
IT
Review request changed

Status: Closed (submitted)

Change Summary:

This change landed and is in the codebase, but I can't track down the exact commit

Loading...