#369 Fixes #368 Add support to jinja templates and multi includes
Closed 4 years ago by sergiomb. Opened 4 years ago by sergiomb.
sergiomb/FedoraReview master  into  master

file modified
+26 -8
@@ -25,6 +25,7 @@ 

  import os.path

  import re

  import shlex

+ import mockbuild.util

  

  from glob import glob

  from subprocess import call, Popen, PIPE, STDOUT, CalledProcessError
@@ -119,7 +120,9 @@ 

  

  def _setup_default_config_opts():

      """ sets up default configuration. """

-     config_opts = {}

+     config_opts = mockbuild.util.TemplatedDictionary()

+     config_opts['__jinja_expand'] = True

+     config_opts['config_paths'] = []

      config_opts['basedir'] = '/var/lib/mock'  # root name is automatically added to this

      config_opts['resultdir'] = '%(basedir)s/%(root)s/result'

      config_opts['cache_topdir'] = '/var/cache/mock'
@@ -375,6 +378,23 @@ 

              raise ReviewError("Can't build x86_64 on i86 host")

          return macros

  

+     def process_include(self, config_file, config_opts):

+         str_mx = re.compile(r'^\s*include\((.*)\)')

+         if os.path.exists(config_file):

+             if config_file in config_opts['config_paths']:

+                 raise exception.ConfigError("Multiple inclusion of %s" % config_file)

+             config_opts['config_paths'].append(config_file)

+             with open(config_file) as f:

+                 content = f.read()

+                 res = str_mx.search(content)

+                 if res is not None:

+                     sub_config_file = eval(res.groups()[0])

+                     sub_content = self.process_include(sub_config_file, config_opts)

+                     content = re.sub(r'^\s*nclude\(.*\)', sub_content, content, count=1)

+             return content

+         else:

+             raise exception.ConfigError("Could not find included config file: %s" % config_file)

+ 

      def _get_root(self):

          """ Return mock's root according to Settings. """

          config = "default"
@@ -388,14 +408,12 @@ 

          )

  

          config_opts = _setup_default_config_opts()

-         with open(path) as f:

-             content = f.read()

-             content = re.sub(r'include\((.*)\)',

-                              r'exec(open(\g<1>).read(), {}, locals())',

-                              content)

-             config = compile(content, path, 'exec')

-         exec(config)

+         #config_opts = mockbuild.util.setup_default_config_opts(None, "unknown",

+         #    "/usr/lib/python3.7/site-packages/mockbuild/")

+         content = self.process_include(path, config_opts)

+         exec(content)

          self.mock_root = config_opts["root"]

+         print ("self.mock_root = %s" % self.mock_root)

          if Settings.uniqueext:

              self.mock_root += Settings.uniqueext

  

A Quick fix , which works I will discuss with mock development a way to use mockbuild.utils for now is now possible for some function so this commit is more a copy of mock code , but I have done some on my own , also I will review my own code with mock development .

@msuchy ccing you - can we have some kind of API for this please?

yes , it is a leftover and can be deleted but comment was left in code to reminder me as a future alternative

config_opts = mockbuild.util.setup_default_config_opts(None, "unknown", "/usr/lib/python3.7/site-packages/mockbuild/")

just not work, unfortunately , I'd like to use it instead set defaults locally ...

ERROR: Exception(/home/sergio/fedora-scm/fedora-review/FedoraReview/5435-smtube/srpm/smtube-19.6.0-1.fc30.src.rpm) Config(fedora-rawhide-x86_64)0 minutes 22 seconds
INFO: Results and/or logs in: /home/sergio/fedora-scm/fedora-review/FedoraReview/5435-smtube/results
ERROR: Command failed: 
--- Logging error ---
Traceback (most recent call last):
File "/usr/lib64/python3.7/logging/__init__.py", line 1025, in emit
msg = self.format(record)
File "/usr/lib64/python3.7/logging/__init__.py", line 869, in format
return fmt.format(record)
File "/usr/lib64/python3.7/logging/__init__.py", line 608, in format
record.message = record.getMessage()
File "/usr/lib64/python3.7/logging/__init__.py", line 369, in getMessage
msg = msg % self.args
TypeError: list indices must be integers or slices, not str
Call stack:
File "/usr/bin/fedora-review", line 25, in <module>
rc = review.run()
File "/usr/lib/python3.7/site-packages/FedoraReview/review_helper.py", line 236, in run
self._do_run(outfile)
File "/usr/lib/python3.7/site-packages/FedoraReview/review_helper.py", line 226, in _do_run
self._do_report(outfile)
File "/usr/lib/python3.7/site-packages/FedoraReview/review_helper.py", line 99, in _do_report
self._run_checks(self.bug.spec_file, self.bug.srpm_file, outfile)
File "/usr/lib/python3.7/site-packages/FedoraReview/review_helper.py", line 117, in _run_checks
self.checks.run_checks(output=output, writedown=not Settings.no_report)
File "/usr/lib/python3.7/site-packages/FedoraReview/checks.py", line 382, in run_checks
run_check(name)
File "/usr/lib/python3.7/site-packages/FedoraReview/checks.py", line 357, in run_check
check.run()
File "/usr/lib/python3.7/site-packages/FedoraReview/plugins/generic_build.py", line 203, in run
Mock.build(self.srpm.filename)
File "/usr/lib/python3.7/site-packages/FedoraReview/mock.py", line 665, in build
self.builddir_cleanup()
File "/usr/lib/python3.7/site-packages/FedoraReview/mock.py", line 792, in builddir_cleanup
paths = glob(os.path.join(self.get_builddir("BUILD"), "*"))
File "/usr/lib/python3.7/site-packages/FedoraReview/mock.py", line 568, in get_builddir
p = self._get_dir(os.path.join("root", self._topdir[1:]))
File "/usr/lib/python3.7/site-packages/FedoraReview/mock.py", line 409, in _get_dir
self._get_root()
File "/usr/lib/python3.7/site-packages/FedoraReview/mock.py", line 394, in _get_root
mockbuild.util.include(path, config_opts)
File "/usr/lib/python3.7/site-packages/mockbuild/trace_decorator.py", line 90, in trace
func=frame.f_code.co_name)
File "/usr/lib/python3.7/site-packages/mockbuild/trace_decorator.py", line 44, in doLog
logger.handle(logger.makeRecord(logger.name, level, *args, **kargs)) 

1 new commit added

  • Make sure that not parse include on comments
4 years ago

@sergiomb Why not directly call util.update_config_from_file?

@churchyard 1) it is not the main task of Mock to provide a library for a general reading of config files :) 2) nevertheless, can you define what API call you would like? I think the the mentioned util.update_config_from_file can squash this PR into one or two lines (OK, maybe three) :).

1) is clear to me :)

2) let's see, that might be enough

Thanks

util.update_config_from_file can squash this PR into one or two lines (OK, maybe three) :).

yes , I tried it in first place but util.update_config_from_file have an os.fork [1] and return sys.exit(0) [2] This prevents using the function in Fedorareview. Or at least I don't know how we catch the exit without exit.
The code is more a less a copy of mock.util , more my own code to improve include function, but could be an exact copy of mock.util

Thanks

[1]

r_pipe, w_pipe = os.pipe() 
if os.fork() == 0:

[2]

sys.exit(0)

OK , I don't know what happened with my previous tests or if it https://pagure.io/FedoraReview/pull-request/371 that fixed something.
Now we have the same error of https://pagure.io/FedoraReview/pull-request/369#comment-100830 in trace_decorator.py .
How we avoid trace_decorator.py ?
Thanks

--- Logging error ---
Traceback (most recent call last):
  File "/usr/lib64/python3.7/logging/__init__.py", line 1025, in emit
    msg = self.format(record)
  File "/usr/lib64/python3.7/logging/__init__.py", line 869, in format
    return fmt.format(record)
  File "/usr/lib64/python3.7/logging/__init__.py", line 608, in format
    record.message = record.getMessage()
  File "/usr/lib64/python3.7/logging/__init__.py", line 369, in getMessage
    msg = msg % self.args
TypeError: list indices must be integers or slices, not str
Call stack:
  File "/usr/bin/fedora-review", line 25, in <module>
    rc = review.run()
  File "/usr/lib/python3.7/site-packages/FedoraReview/review_helper.py", line 236, in run
    self._do_run(outfile)
  File "/usr/lib/python3.7/site-packages/FedoraReview/review_helper.py", line 226, in _do_run
    self._do_report(outfile)
  File "/usr/lib/python3.7/site-packages/FedoraReview/review_helper.py", line 99, in _do_report
    self._run_checks(self.bug.spec_file, self.bug.srpm_file, outfile)
  File "/usr/lib/python3.7/site-packages/FedoraReview/review_helper.py", line 117, in _run_checks
    self.checks.run_checks(output=output, writedown=not Settings.no_report)
  File "/usr/lib/python3.7/site-packages/FedoraReview/checks.py", line 382, in run_checks
    run_check(name)
  File "/usr/lib/python3.7/site-packages/FedoraReview/checks.py", line 357, in run_check
    check.run()
  File "/usr/lib/python3.7/site-packages/FedoraReview/plugins/generic_build.py", line 203, in run
    Mock.build(self.srpm.filename)
  File "/usr/lib/python3.7/site-packages/FedoraReview/mock.py", line 687, in build
    self.builddir_cleanup()
  File "/usr/lib/python3.7/site-packages/FedoraReview/mock.py", line 814, in builddir_cleanup
    paths = glob(os.path.join(self.get_builddir("BUILD"), "*"))
  File "/usr/lib/python3.7/site-packages/FedoraReview/mock.py", line 590, in get_builddir
    p = self._get_dir(os.path.join("root", self._topdir[1:]))
  File "/usr/lib/python3.7/site-packages/FedoraReview/mock.py", line 431, in _get_dir
    self._get_root()
  File "/usr/lib/python3.7/site-packages/FedoraReview/mock.py", line 411, in _get_root
    mockbuild.util.update_config_from_file(config_opts, path, None)
  File "/usr/lib/python3.7/site-packages/mockbuild/trace_decorator.py", line 90, in trace
    func=frame.f_code.co_name)
  File "/usr/lib/python3.7/site-packages/mockbuild/trace_decorator.py", line 44, in doLog
    logger.handle(logger.makeRecord(logger.name, level, *args, **kargs))

just for curiosity, ipython3 crashed 100% on the times on Fedora 30 with [1] .
[2] seems to be the code that is need for FedoraReview .
maybe we need add config_opts['__jinja_expand'] = True I will test it soon .

[1]

In [1]: import mockbuild.util                                                                                                                   

In [2]: from sysconfig import get_paths                                                                                                         

In [3]: config_opts = mockbuild.util.setup_default_config_opts(None, "unknown", get_paths()['purelib'])                                         

In [4]: config_opts['config_paths'] = []                                                                                                        


In [5]: mockbuild.util.update_config_from_file(config_opts,"/etc/mock/fedora-31-x86_64-rpmfusion_free.cfg", None)                               
An exception has occurred, use %tb to see the full traceback.

SystemExit: 0

/usr/lib/python3.7/site-packages/IPython/core/interactiveshell.py:3275: UserWarning: To exit: use 'exit', 'quit', or Ctrl-D.
  warn("To exit: use 'exit', 'quit', or Ctrl-D.", stacklevel=1)

In [6]: %bt       

[2]

import mockbuild.util                                                                                                                   
logging.getLogger("trace").propagate = 0
from sysconfig import get_paths                                                                                                         
config_opts = mockbuild.util.setup_default_config_opts(None, "unknown", get_paths()['purelib'])                                         
config_opts['config_paths'] = []  
mockbuild.util.update_config_from_file(config_opts,"/etc/mock/fedora-31-x86_64-rpmfusion_free.cfg", None)     

Pull-Request has been closed by sergiomb

4 years ago

@sergiomb Why not directly call util.update_config_from_file?

Thanks, I follow the suggestion and it is done here https://pagure.io/FedoraReview/pull-request/374

Metadata