Add changesemail and adapt changesgenerate behavior

luc14n0 · luc14n0 · commit b8719ab5915e · 2023-04-14T03:10:09.000-03:00
Modify default behavior of changesgenerate to add packager's name AND
email by default, instead of only the former, in the generated changes
entry header. This also adds changesemail, a new parameter for _service
files, that must be used along with changesauthor whenever configuring
them manually is needed.

Signed-off-by: Luciano Santos &lt;luc14n0@opensuse.org&gt;
diff --git a/TarSCM/changes.py b/TarSCM/changes.py
@@ -176,7 +176,7 @@ def write_changes_revision(self, url, outdir, new_revision):
         if changed:
             xml_tree.write(os.path.join(outdir, "_servicedata"))
 
-    def write_changes(self, changes_filename, changes, version, author):
+    def write_changes(self, changes_filename, changes, version, author, email):
         """Add changes to given *.changes file."""
         if changes is None:
             logging.debug(
@@ -196,7 +196,7 @@ def write_changes(self, changes_filename, changes, version, author):
         dtime = datetime.datetime.utcnow().strftime('%a %b %d %H:%M:%S UTC %Y')
 
         text = '-' * 67 + '\n'
-        text += "%s - %s\n" % (dtime, author)
+        text += "%s - %s <%s>\n" % (dtime, author, email)
         text += '\n'
         text += "- Update to version %s:\n" % version
         for line in changes:
@@ -231,25 +231,71 @@ def get_changesauthor(self, args):
             return args.changesauthor
 
         # return changesauthor if set by osc
-        if os.getenv('VC_MAILADDR'):
-            logging.debug("Found changesauthor in VC_MAILADDR='%s'",
-                          os.environ['VC_MAILADDR'])
-            return os.environ['VC_MAILADDR']
+        if os.getenv('VC_REALNAME'):
+            logging.debug("Found changesauthor in VC_REALNAME='%s'",
+                          os.environ['VC_REALNAME'])
+            return os.environ['VC_REALNAME']
 
         # return default changesauthor if running on server side
         if os.getenv('OBS_SERVICE_DAEMON'):
             logging.debug("Running in daemon mode. Using DEFAULT_AUHTOR='%s'",
                           Cli.DEFAULT_AUTHOR)
             return Cli.DEFAULT_AUTHOR
 
-        # exit if running locally (non server mode) and now changesauthor
-        # could be determined
+        # exit if running locally (non server mode) and no changesauthor could
+        # be determined, hint user on what to do
+        raise SystemExit(
+            """No 'changesauthor' has been defined! You can do it by:\n\n"""
+            """Configuring:\n"""
+            """    * The 'realname' entry for the default API section """
+            """(api.opensuse.org) -- or whatever OBS\n"""
+            """      instance you're working with -- in your OSCRC file """
+            """with:\n\n"""
+            """        osc config "https://api.opensuse.org" realname John """
+            """"Doe"\n\n"""
+            """    * The '<param name="changesauthor">John Doe</param> tag """
+            """nested under the obs_scm/tar_scm\n"""
+            """      service tag in your _service file.\n\n"""
+            """Passing:\n"""
+            """    * '--changesauthor "John Doe"' on the CLI, when using """
+            """the obs_scm/tar_scm script manually.\n"""
+        )
+
+    def get_changesemail(self, args):
+        # return changesemail if given as cli option
+        if args.changesemail:
+            logging.debug("Found changesemail in args.changesemail='%s'",
+                          args.changesemail)
+            return args.changesemail
+
+        # return changesemail if set by osc
+        if os.getenv('VC_MAILADDR'):
+            logging.debug("Found changesemail in VC_MAILADDR='%s'",
+                          os.environ['VC_MAILADDR'])
+            return os.environ['VC_MAILADDR']
+
+        # return default changesemail if running on server side
+        if os.getenv('OBS_SERVICE_DAEMON'):
+            logging.debug("Running in daemon mode. Using DEFAULT_EMAIL='%s'",
+                          Cli.DEFAULT_EMAIL)
+            return Cli.DEFAULT_EMAIL
+
+        # exit if running locally (non server mode) and no changesemail could
+        # be determined, hint user on what to do
         raise SystemExit(
-            """No changesauthor defined!\n"""
-            """You can define it by:\n"""
-            """ * configure 'email=' in ~/.config/osc/oscrc """
-            """in your default api section\n"""
-            """ * configure <param name="changesauthor">"""
-            """...</param> in your _service file\n"""
-            """ * using '--changesauthor' on the cli\n"""
+            """No 'changesemail' has been defined! You can do it by:\n\n"""
+            """Configuring:\n"""
+            """    * The 'email' entry for the default API section """
+            """(api.opensuse.org) -- or whatever OBS\n"""
+            """      instance you're working with -- in your OSCRC file """
+            """with:\n\n"""
+            """        osc config "https://api.opensuse.org" email """
+            """"jdoe@example.com"\n\n"""
+            """    * The '<param name="changesemail">jdoe@example.com"""
+            """</param> tag nested under the\n"""
+            """      obs_scm/tar_scm service tag in your _service file.\n\n"""
+            """Passing:\n"""
+            """    * '--changesemail "jdoe@example.com"' on the CLI, when """
+            """using the obs_scm/tar_scm script\n"""
+            """      manually.\n"""
         )
diff --git a/TarSCM/cli.py b/TarSCM/cli.py
@@ -45,7 +45,8 @@ def check_locale(loc):
 class Cli():
     # pylint: disable=too-few-public-methods
     # pylint: disable=too-many-instance-attributes
-    DEFAULT_AUTHOR = 'obs-service-tar-scm@invalid'
+    DEFAULT_AUTHOR = 'service_run'
+    DEFAULT_EMAIL = 'obs-service-tar-scm@invalid'
     outdir = None
 
     def __init__(self):
@@ -120,10 +121,15 @@ def parse_args(self, options):
                                  'parent revision (see changesrevision).')
         parser.add_argument('--changesauthor',
                             help='The author of the changes file entry to be '
-                                 'written, defaults to first email entry in '
-                                 'osc rc files or "%s" '
-                                 'if there is no ~/.oscrc found.' %
+                                 'written. Defaults to VC_REALNAME env '
+                                 'variable, set by osc. Or "%s", otherwise.' %
                             self.DEFAULT_AUTHOR)
+        parser.add_argument('--changesemail',
+                            help='The author\'s email of the changes file '
+                                 'entry to be written. Defaults to VC_MAILADDR'
+                                 ' env variable, set by osc. Or "%s", '
+                                 'otherwise.' %
+                            self.DEFAULT_EMAIL)
         parser.add_argument('--subdir', default='',
                             help='Package just a subdirectory of the sources')
         parser.add_argument('--submodules',
diff --git a/TarSCM/tasks.py b/TarSCM/tasks.py
@@ -283,8 +283,9 @@ def _dstname(self, scm_object, version):
 
     def _process_changes(self, args, ver, changesversion, detected_changes):
         changesauthor = self.changes.get_changesauthor(args)
+        changesemail = self.changes.get_changesemail(args)
 
-        logging.debug("AUTHOR: %s", changesauthor)
+        logging.debug("AUTHOR <EMAIL>: %s <%s>", changesauthor, changesemail)
 
         if not ver:
             args.version = "_auto_"
@@ -296,7 +297,9 @@ def _process_changes(self, args, ver, changesversion, detected_changes):
             shutil.copy(filename, new_changes_file)
             self.changes.write_changes(new_changes_file,
                                        detected_changes['lines'],
-                                       changesversion, changesauthor)
+                                       changesversion,
+                                       changesauthor,
+                                       changesemail)
         self.changes.write_changes_revision(args.url, args.outdir,
                                             detected_changes['revision'])
 
diff --git a/tar_scm.service.in b/tar_scm.service.in
@@ -195,7 +195,10 @@ which get maintained in the SCM. Can be used multiple times.</description>
     <allowedvalue>disable</allowedvalue>
   </parameter>
   <parameter name="changesauthor">
-    <description>Specify author of the changes file entry to be written.  Defaults to first email entry in ~/.oscrc, or "obs-service-tar-scm@invalid" if there is no .oscrc found.</description>
+    <description>Specify author of the changes file entry to be written. Defaults to VC_REALNAME environment variable, set by osc, or "service_run" otherwise.</description>
+  </parameter>
+  <parameter name="changesemail">
+    <description>Specify author's email of the changes file entry to be written. Defaults to VC_MAILADDR environment variable, set by osc, or "obs-service-tar-scm@invalid" otherwise.</description>
   </parameter>
   <parameter name="locale">
     <description>DEPRECATED - Please use "encoding" instead. Set locale while execution of service</description>
diff --git a/tests/gitsvntests.py b/tests/gitsvntests.py
@@ -17,7 +17,8 @@ class GitSvnTests(CommonTests):
 
     def _tar_scm_changesgenerate(self, mode, **kwargs):
         self.tar_scm_std(
-            '--changesauthor', 'a@b.c',
+            '--changesauthor', 'spam',
+            '--changesemail', 'a@b.c',
             '--changesgenerate', mode,
             **kwargs
         )
@@ -90,13 +91,13 @@ def test_changesgenerate_new_commit_no_changes_file(self):  # pylint: disable=C0
         self._tar_scm_changesgenerate('enable')
         self._check_servicedata(revision=3)
 
-    def _new_change_entry_regexp(self, author, changes):  # pylint: disable=R0201
+    def _new_change_entry_regexp(self, author, email, changes):  # pylint: disable=R0201
         return textwrap.dedent("""\
           ^-------------------------------------------------------------------
-          \w{3} \w{3} [ \d]\d \d\d:\d\d:\d\d [A-Z]{3} 20\d\d - %s
+          \w{3} \w{3} [ \d]\d \d\d:\d\d:\d\d [A-Z]{3} 20\d\d - %s <%s>
 
           %s
-          """) % (author, changes)
+          """) % (author, email, changes)
 
     def _check_changes(self, orig_changes, expected_changes_regexp):
         new_changes_file = os.path.join(self.outdir, 'pkg.changes')
@@ -129,7 +130,7 @@ def _write_servicedata(self, rev):
                 </service>
               </servicedata>""" % (self.fixtures.repo_url, self.changesrevision(rev))))
 
-    def _test_changesgenerate_new_commit_and_changes_file(self, author=None):  # pylint: disable=C0103
+    def _test_changesgenerate_new_commit_and_changes_file(self, author=None, email=None):  # pylint: disable=C0103
         self._write_servicedata(2)
         orig_changes = self._write_changes_file()
         self.fixtures.create_commits(3)
@@ -138,7 +139,10 @@ def _test_changesgenerate_new_commit_and_changes_file(self, author=None):  # pyl
         tar_scm_args = self.tar_scm_args()
 
         if author is not None:
-            tar_scm_args += ['--changesauthor', self.fixtures.user_email]
+            tar_scm_args += ['--changesauthor', self.fixtures.user_name]
+
+        if email is not None:
+            tar_scm_args += ['--changesemail', self.fixtures.user_email]
 
         print("XXXX 2")
         self.tar_scm_std(*tar_scm_args)
@@ -149,9 +153,11 @@ def _test_changesgenerate_new_commit_and_changes_file(self, author=None):  # pyl
         rev = self.changesrevision(rev, abbrev=True)
 
         print("XXXX 4")
-        expected_author = author or 'obs-service-tar-scm@invalid'
+        expected_author = author or 'geeko'
+        expected_email = email or 'obs-service-tar-scm@invalid'
         expected_changes_regexp = self._new_change_entry_regexp(
             expected_author,
+            expected_email,
             textwrap.dedent("""\
               - Update to version 0.6.%s:
                 \* 5
@@ -170,7 +176,8 @@ def test_changesgenerate_new_commit_and_changes_file_no_version(self):  # pylint
         tar_scm_args = [
             '--changesgenerate', 'enable',
             '--version', '',
-            '--changesauthor', self.fixtures.user_email
+            '--changesauthor', self.fixtures.user_name,
+            '--changesemail', self.fixtures.user_email
         ]
         self.tar_scm_std(*tar_scm_args)
 
@@ -179,9 +186,11 @@ def test_changesgenerate_new_commit_and_changes_file_no_version(self):  # pylint
         rev = self.changesrevision(rev, abbrev=True)
         ver_regex = self.changesregex(rev)
 
-        expected_author = self.fixtures.user_email
+        expected_author = self.fixtures.user_name
+        expected_email = self.fixtures.user_email
         expected_changes_regexp = self._new_change_entry_regexp(
             expected_author,
+            expected_email,
             textwrap.dedent("""\
               - Update to version %s:
                 \* 5
@@ -202,16 +211,19 @@ def test_changesgenerate_new_commit_and_changes_file_with_subdir(self):   # pyli
 
         tar_scm_args += [
             '--subdir', 'another_subdir',
-            '--changesauthor', self.fixtures.user_email,
+            '--changesauthor', self.fixtures.user_name,
+            '--changesemail', self.fixtures.user_email
         ]
 
         self.tar_scm_std(*tar_scm_args)
 
         self._check_servicedata(revision=rev, expected_dirents=3)
 
-        expected_author = self.fixtures.user_email
+        expected_author = self.fixtures.user_name
+        expected_email = self.fixtures.user_email
         expected_changes_regexp = self._new_change_entry_regexp(
             expected_author,
+            expected_email,
             textwrap.dedent("""\
               - Update to version 0.6.%s:
                 \* 8
@@ -236,16 +248,19 @@ def test_changesgenerate_old_servicedata(self):   # pylint: disable=C0103
         tar_scm_args = self.tar_scm_args()
 
         tar_scm_args += [
-            '--changesauthor', self.fixtures.user_email,
+            '--changesauthor', self.fixtures.user_name,
+            '--changesemail', self.fixtures.user_email
         ]
 
         self.tar_scm_std(*tar_scm_args)
 
         self._check_servicedata(revision=rev, expected_dirents=3)
 
-        expected_author = self.fixtures.user_email
+        expected_author = self.fixtures.user_name
+        expected_email = self.fixtures.user_email
         expected_changes_regexp = self._new_change_entry_regexp(
             expected_author,
+            expected_email,
             textwrap.dedent("""\
               - Update to version 0.6.%s:
                 \* 5
diff --git a/tests/gittests.py b/tests/gittests.py
@@ -326,9 +326,9 @@ def test_changesgenerate_unicode(self):
         tar_scm_args = self.tar_scm_args()
 
         tar_scm_args += [
-            '--changesauthor', self.fixtures.user_email,
+            '--changesauthor', self.fixtures.user_name,
+            '--changesemail', self.fixtures.user_email
         ]
-
         self.tar_scm_std(*tar_scm_args)
 
         self._check_servicedata(revision=rev, expected_dirents=3)
diff --git a/tests/unittestcases.py b/tests/unittestcases.py
@@ -133,17 +133,17 @@ def test_changes_get_chga_args(self):
         '''Test if getting changesauthor from cli args works'''
         chg                 = Changes()
         cli                 = copy.copy(self.cli)
-        cli.changesauthor   = 'args@example.com'
+        cli.changesauthor   = 'geeko'
         author              = chg.get_changesauthor(cli)
-        self.assertEqual(author, 'args@example.com')
+        self.assertEqual(author, 'geeko')
 
     def test_changes_get_chga_oscrc(self):
         '''Test if getting changesauthor from .oscrc works'''
-        os.environ["VC_MAILADDR"] = 'devel@example.com'
+        os.environ["VC_REALNAME"] = 'geeko'
         chg                 = Changes()
         author              = chg.get_changesauthor(self.cli)
-        self.assertEqual(author, 'devel@example.com')
-        os.environ["VC_MAILADDR"] = ''
+        self.assertEqual(author, 'geeko')
+        os.environ["VC_REALNAME"] = ''
 
     def test_changes_get_chga_default(self):
         '''Test if getting default changesauthor if running inside OBS'''
@@ -153,7 +153,34 @@ def test_changes_get_chga_default(self):
         chg                 = Changes()
         author              = chg.get_changesauthor(self.cli)
         os.environ['HOME']  = home
-        self.assertEqual(author, 'obs-service-tar-scm@invalid')
+        self.assertEqual(author, 'geeko')
+        os.environ['OBS_SERVICE_DAEMON'] = "0"
+
+    def test_changes_get_chge_args(self):
+        '''Test if getting changesemail from cli args works'''
+        chg                 = Changes()
+        cli                 = copy.copy(self.cli)
+        cli.changesemail    = 'args@example.com'
+        email               = chg.get_changesemail(cli)
+        self.assertEqual(email, 'args@example.com')
+
+    def test_changes_get_chge_oscrc(self):
+        '''Test if getting changesemail from osc works'''
+        os.environ["VC_MAILADDR"] = 'devel@example.com'
+        chg                 = Changes()
+        email               = chg.get_changesemail(self.cli)
+        self.assertEqual(email, 'devel@example.com')
+        os.environ["VC_MAILADDR"] = ''
+
+    def test_changes_get_chge_default(self):
+        '''Test if getting default changesemail if running inside OBS'''
+        os.environ['OBS_SERVICE_DAEMON'] = "1"
+        home                = os.environ['HOME']
+        os.environ['HOME']  = '/nir/va/na'
+        chg                 = Changes()
+        email              = chg.get_changesemail(self.cli)
+        os.environ['HOME']  = home
+        self.assertEqual(email, 'obs-service-tar-scm@invalid')
         os.environ['OBS_SERVICE_DAEMON'] = "0"
 
     def test_git_repoc_hash_wo_subdir(self):
