Make "./pants changed" output correct results when BUILD files are mo…

…dified (pantsbuild#4282)

### Problem

pantsbuild#3933 

Currently with v2 engine, if a dir has more than 1 BUILD file, changing only 1 of them will cause "./pants changed" output all targets in that dir, even though some targets are not defined in the changed BUILD file.

The reason is here: https://github.com/pantsbuild/pants/blob/master/src/python/pants/engine/legacy/address_mapper.py#L53.

In is_declaring_file method, since Address object does not have information about declaring BUILD file, the result is not accurate.

### Solution

With pantsbuild@257a622,
Now addresses passed to that method are BuildFileAddress objects, whose "rel_path" field is the path to its declaring BUILD file. I still kept the old logic in case an Address object is passed to this method. After pantsbuild#3925 is fixed, BuildFileAddress and Address will become 1 class, thus we can eliminate the extra logic.

src/python/pants/engine/legacy/address_mapper.py

@@ -51,13 +51,19 @@ def scan_build_files(self, base_path):
 
   @staticmethod
   def is_declaring_file(address, file_path):
-    # NB: this will cause any BUILD file, whether it contains the address declaration or not to be
-    # considered the one that declared it. That's ok though, because the spec path should be enough
-    # information for debugging most of the time.
-    #
-    # We could call into the engine to ask for the file that declared the address.
-    return (os.path.dirname(file_path) == address.spec_path and
-            BuildFile._is_buildfile_name(os.path.basename(file_path)))
+    if not BuildFile._is_buildfile_name(os.path.basename(file_path)):
+      return False
+
+    try:
+      # A precise check for BuildFileAddress
+      return address.rel_path == file_path
+    except AttributeError:
+      # NB: this will cause any BUILD file, whether it contains the address declaration or not to be
+      # considered the one that declared it. That's ok though, because the spec path should be enough
+      # information for debugging most of the time.
+      #
+      # TODO: remove this after https://github.com/pantsbuild/pants/issues/3925 lands
+      return os.path.dirname(file_path) == address.spec_path
 
   def addresses_in_spec_path(self, spec_path):
     return self.scan_specs([SiblingAddresses(spec_path)])

tests/python/pants_test/engine/legacy/test_address_mapper.py

@@ -12,7 +12,7 @@
 
 from pants.base.specs import SiblingAddresses, SingleAddress
 from pants.bin.engine_initializer import EngineInitializer
-from pants.build_graph.address import Address
+from pants.build_graph.address import Address, BuildFileAddress
 from pants.build_graph.address_mapper import AddressMapper
 from pants.engine.legacy.address_mapper import LegacyAddressMapper
 from pants.util.contextutil import temporary_dir
@@ -102,6 +102,12 @@ def test_is_declaring_file(self):
     self.assertTrue(mapper.is_declaring_file(Address('path', 'name'), 'path/BUILD.suffix'))
     self.assertFalse(mapper.is_declaring_file(Address('path', 'name'), 'path/not_a_build_file'))
     self.assertFalse(mapper.is_declaring_file(Address('path', 'name'), 'differing-path/BUILD'))
+    self.assertFalse(mapper.is_declaring_file(
+      BuildFileAddress(target_name='name', rel_path='path/BUILD.new'),
+      'path/BUILD'))
+    self.assertTrue(mapper.is_declaring_file(
+      BuildFileAddress(target_name='name', rel_path='path/BUILD'),
+      'path/BUILD'))
 
   def test_addresses_in_spec_path(self):
     with temporary_dir() as build_root:

tests/python/pants_test/engine/legacy/test_changed_integration.py

@@ -370,6 +370,17 @@ def test_changed_diffspec_and_files(self):
          'src/python/python_targets/test_unclaimed_src.py'}
       )
 
+  @ensure_engine
+  def test_changed_with_multiple_build_files(self):
+    new_build_file = 'src/python/python_targets/BUILD.new'
+
+    with create_isolated_git_repo() as worktree:
+      touch(os.path.join(worktree, new_build_file))
+      pants_run = self.run_pants(['changed'])
+
+      self.assert_success(pants_run)
+      self.assertEqual(pants_run.stdout_data.strip(), '')
+
   # Following 4 tests do not run in isolated repo because they don't mutate working copy.
   def test_changed(self):
     self.assert_changed_new_equals_old([])