From b0a5f881bd67f1e6c3927f20eb3c0f247e7cc448 Mon Sep 17 00:00:00 2001
From: Joshua Watt <JPEWhacker@gmail.com>
Date: Thu, 27 Oct 2022 16:22:52 -0500
Subject: [PATCH] executor: Skip line mapping for special Gerrit files

If the file being comment on is the Gerrit special file starting with
"/" (e.g. /COMMIT_MSG), no line mapping transformation should be done,
otherwise strange errors like:

  Job: unable to map line for file comments:
  stderr: 'fatal: '/COMMIT_MSG' is outside repository at '...'

will show up after the job has run.

Change-Id: Id89041dc7d8bf3f6c956d85b38355053ff0fd707
---
 .../org_project/playbooks/file-comments.yaml  |  3 +++
 tests/unit/test_gerrit.py                     | 26 ++++++++++++++-----
 zuul/executor/server.py                       |  5 ++++
 3 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/tests/fixtures/config/gerrit-file-comments/git/org_project/playbooks/file-comments.yaml b/tests/fixtures/config/gerrit-file-comments/git/org_project/playbooks/file-comments.yaml
index 53a87d8927..cd4d9ec03c 100644
--- a/tests/fixtures/config/gerrit-file-comments/git/org_project/playbooks/file-comments.yaml
+++ b/tests/fixtures/config/gerrit-file-comments/git/org_project/playbooks/file-comments.yaml
@@ -15,6 +15,9 @@
               missingfile.txt:
                 - line: 89
                   message: does not exist
+              /COMMIT_MSG:
+                - line: 1
+                  message: commit message comment
     - name: Another task using zuul_return
       zuul_return:
         data:
diff --git a/tests/unit/test_gerrit.py b/tests/unit/test_gerrit.py
index e98cb80b5f..2e3057af64 100644
--- a/tests/unit/test_gerrit.py
+++ b/tests/unit/test_gerrit.py
@@ -396,10 +396,24 @@ class TestFileComments(AnsibleZuulTestCase):
                          'SUCCESS')
         self.assertEqual(self.getJobFromHistory('file-comments-error').result,
                          'SUCCESS')
-        self.assertEqual(len(A.comments), 6)
+        self.assertEqual(len(A.comments), 7)
         comments = sorted(A.comments, key=lambda x: (x['file'], x['line']))
         self.assertEqual(
             comments[0],
+            {
+                'file': '/COMMIT_MSG',
+                'line': 1,
+                'message': 'commit message comment',
+                'reviewer': {
+                    'email': 'zuul@example.com',
+                    'name': 'Zuul',
+                    'username': 'jenkins'
+                },
+            },
+        )
+
+        self.assertEqual(
+            comments[1],
             {
                 'file': 'otherfile.txt',
                 'line': 21,
@@ -414,7 +428,7 @@ class TestFileComments(AnsibleZuulTestCase):
         )
 
         self.assertEqual(
-            comments[1],
+            comments[2],
             {
                 "file": "path/to/file.py",
                 "line": 2,
@@ -428,7 +442,7 @@ class TestFileComments(AnsibleZuulTestCase):
         )
 
         self.assertEqual(
-            comments[2],
+            comments[3],
             {
                 "file": "path/to/file.py",
                 "line": 21,
@@ -445,7 +459,7 @@ class TestFileComments(AnsibleZuulTestCase):
         )
 
         self.assertEqual(
-            comments[3],
+            comments[4],
             {
                 'file': 'path/to/file.py',
                 'line': 42,
@@ -459,7 +473,7 @@ class TestFileComments(AnsibleZuulTestCase):
         )
 
         self.assertEqual(
-            comments[4],
+            comments[5],
             {
                 "file": "path/to/file.py",
                 "line": 42,
@@ -475,7 +489,7 @@ class TestFileComments(AnsibleZuulTestCase):
             }
         )
 
-        self.assertEqual(comments[5],
+        self.assertEqual(comments[6],
                          {'file': 'path/to/file.py',
                           'line': 82,
                           'message': 'line too short',
diff --git a/zuul/executor/server.py b/zuul/executor/server.py
index 9c7ed158c9..7a755177b9 100644
--- a/zuul/executor/server.py
+++ b/zuul/executor/server.py
@@ -1586,6 +1586,11 @@ class AnsibleJob(object):
 
         new_lines = {}
         for (filename, lineno) in lines:
+            # Gerrit has several special file names (like /COMMIT_MSG) that
+            # start with "/" and should not have mapping done on them
+            if filename[0] == "/":
+                continue
+
             try:
                 new_lineno = repo.mapLine(commit, filename, lineno)
             except Exception as e: