Do not return directories from BUILD file's globs implementation

Review Request #3590 — Created March 21, 2016 and submitted

tabishev
pants
tabishev/jvmapp_hash_issue
3068
pants-reviews
benjyw, nhoward_tw, stuhood, zundel

I've discovered fingerprint method fails on jvm_app targets because of pants globs implementation returns folders which are not hashable. I think it's fair enough to return only files in globs/rglobs/zglobs implementations for BUILD files. This PR contains tests for both jvm_app fingerprint and globs behaviour + fix for it.

https://travis-ci.org/ttim/pants/builds/117934043

  • 0
  • 0
  • 2
  • 0
  • 2
Description From Last Updated
  1. Ship It!
  2. 
      
  1. Ship It!
  2. You could add another test that creates an empty directory and makes sure the fingerprint is unchanged.

  3. 
      
  1. Ship It!
  2. 
      
  1. 
      
  2. I feel like this should be in the globs implementation, not here. "files_calculator" sounds like something that should already only return files.

    1. @Benjy: I'm not sure I agree. We know that this kind of processing isn't interested in directories, but isn't it reasonable to think that some other operation would be?

    2. Maybe, but in that case files_calculator should be paths_calculator or something. The names need to reflect reality...

    3. I agree, changed a place where we are preparing files_calculator to return only actual files, not folders.

  3. 
      
  1. 
      
  2. Ah, this is exactly the right place for this. In the future if we need directories we can create a paths_calculator() method.

  3. Thanks for the fixes!

  1. Ship It!
  2. 
      
Review request changed

Status: Closed (submitted)

Change Summary:

Merged as 2af4f6c53f0a7391111d7949c44f9a59cd1f407d

Loading...