From a7c33b428f4c383a74dfc2cbe99aa91212934c70 Mon Sep 17 00:00:00 2001
From: Benjy Weinberger <benjyw@gmail.com>
Date: Tue, 4 Aug 2020 14:52:47 -0700
Subject: [PATCH] Address code review comments and add test

# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]

# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
---
 .../protobuf/python/additional_fields.py       |  3 +--
 .../backend/codegen/protobuf/python/rules.py   |  2 +-
 .../protobuf/python/rules_integration_test.py  | 18 +++++++++++++++++-
 3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/src/python/pants/backend/codegen/protobuf/python/additional_fields.py b/src/python/pants/backend/codegen/protobuf/python/additional_fields.py
index ce779cc6017..3328391368c 100644
--- a/src/python/pants/backend/codegen/protobuf/python/additional_fields.py
+++ b/src/python/pants/backend/codegen/protobuf/python/additional_fields.py
@@ -11,13 +11,12 @@ class ProtobufPythonInterpreterCompatibility(PythonInterpreterCompatibility):
 
 
 class PythonSourceRootField(StringField):
-    """The source root to generate python sources under.
+    """The source root to generate Python sources under.
 
     If unspecified, the source root the protobuf_library is under will be used.
     """
 
     alias = "python_source_root"
-    default = None
 
 
 def rules():
diff --git a/src/python/pants/backend/codegen/protobuf/python/rules.py b/src/python/pants/backend/codegen/protobuf/python/rules.py
index 269335a5d33..7ca9c84b84f 100644
--- a/src/python/pants/backend/codegen/protobuf/python/rules.py
+++ b/src/python/pants/backend/codegen/protobuf/python/rules.py
@@ -107,7 +107,7 @@ async def generate_python_from_protobuf(
     )
 
     # We must do some path manipulation on the output digest for it to look like normal sources,
-    # including adding back the original source root.
+    # including adding back a source root.
     py_source_root = request.protocol_target.get(PythonSourceRootField).value
     if py_source_root:
         # Verify that the python source root specified by the target is in fact a source root.
diff --git a/src/python/pants/backend/codegen/protobuf/python/rules_integration_test.py b/src/python/pants/backend/codegen/protobuf/python/rules_integration_test.py
index fd3f6a22e5f..0f15c6633d5 100644
--- a/src/python/pants/backend/codegen/protobuf/python/rules_integration_test.py
+++ b/src/python/pants/backend/codegen/protobuf/python/rules_integration_test.py
@@ -150,7 +150,7 @@ def test_generates_python(self) -> None:
             expected_files=["tests/protobuf/test_protos/f_pb2.py"],
         )
 
-    def test_top_level_source_root(self) -> None:
+    def test_top_level_proto_root(self) -> None:
         self.create_file(
             "protos/f.proto",
             dedent(
@@ -166,6 +166,22 @@ def test_top_level_source_root(self) -> None:
             "protos", source_roots=["/"], expected_files=["protos/f_pb2.py"]
         )
 
+    def test_top_level_python_source_root(self) -> None:
+        self.create_file(
+            "src/proto/protos/f.proto",
+            dedent(
+                """\
+                syntax = "proto2";
+
+                package protos;
+                """
+            ),
+        )
+        self.add_to_build_file("src/proto/protos", "protobuf_library(python_source_root='.')")
+        self.assert_files_generated(
+            "src/proto/protos", source_roots=["/", "src/proto"], expected_files=["protos/f_pb2.py"]
+        )
+
     def test_bad_python_source_root(self) -> None:
         self.create_file(
             "src/protobuf/dir1/f.proto",