PR#390: Add config options to parse the base module (e.g. platform) stream from the dist-git branch and apply a buildrequire override - rpkg

rpkg

#390 Add config options to parse the base module (e.g. platform) stream from the dist-git branch and apply a buildrequire override

Merged 5 years ago by lsedlar. Opened 5 years ago by mprahl.

Unknown source mbs-override-dep-by-branch into master

Add config options to parse the base module (e.g. platform) stream from the dist-git branch and apply a buildrequire override

mprahl • 5 years ago

80a3d1f

pyrpkg/cli.py

file modified

+35 -3

		`@@ -1974,15 +1974,47 @@`
		`self.module_validate_config()`
		`scm_url, branch = self.cmd.module_get_scm_info(`
		`self.args.scm_url, self.args.branch)`
		`+`
		`+ buildrequires = self.args.buildrequires or []`
		`+ branch_search = None`
		`+ try:`
		`+ # The regex to parse the base module stream override from the dist-git branch name.`
		+ # For example, if you had a branch called `10-fedora-29.0.1` and you wanted to
		+ # parse `f29.0.1` from that as the stream to use for the base module, you could
		+ # have a regex of `(f)(?:edora)(?:\-)(\d+\.\d+\.\d+)$`. Then after combining
		+ # the capture groups, you'd get the desired result of `f29.0.1`.
		`+ branch_bm_regex = self.config.get(`
		`+ self.config_section, 'base_module_stream_regex_from_branch')`
		`+ # The base module (e.g. platform) to apply the buildrequire override to if the parsing`
		`+ # of the branch name is successfull`
		`+ bm = self.config.get(self.config_section, 'base_module')`
		`+ except configparser.NoOptionError:`
		`+ pass`
		`+ else:`
		`+ branch_search = re.search(branch_bm_regex, branch)`
		`+`
		`+ if branch_search:`
		`+ # Concatenate all the groups together to get the desired stream.`
		`+ # This approach is taken in case there are sections to ignore.`
		+ # For instance, if we need to parse `f27.0.0` from `fedora-27.0.0`.
		`+ bm_stream = ''.join(branch_search.groups())`
		`+ # Don't override the base module buildrequire if the user`
		+ # manually already overrode it with the `--buildrequire` argument
		`+ if not any(br[0] == bm for br in buildrequires):`
		`+ buildrequires.append((bm, bm_stream))`
		`+ if not self.args.q:`
		`+ print('Added the buildrequire override "{0}" based on the module branch name'`
		`+ .format('{0}:{1}'.format(bm, bm_stream)))`
		`+`
		`auth_method, oidc_id_provider, oidc_client_id, oidc_client_secret, \`
		`oidc_scopes = self.module_get_auth_config()`

		`if not self.args.q:`
		`print('Submitting the module build...')`
		`build_ids = self._cmd.module_submit_build(`
		`- scm_url, branch, auth_method, self.args.buildrequires,`
		`- self.args.requires, self.args.optional, oidc_id_provider,`
		`- oidc_client_id, oidc_client_secret, oidc_scopes)`
		`+ scm_url, branch, auth_method, buildrequires, self.args.requires,`
		`+ self.args.optional, oidc_id_provider, oidc_client_id,`
		`+ oidc_client_secret, oidc_scopes)`
		`if self.args.watch:`
		`self.module_watch_build(build_ids)`
		`elif not self.args.q:`

tests/test_cli.py

file modified

+86

		`@@ -2106,6 +2106,92 @@`
		`assert reqs['platform'] == ['f29']`

		`@patch('requests.get')`
		`+ @patch('openidc_client.OpenIDCClient.send_request')`
		`+ def test_module_build_override_from_branch(self, mock_oidc_req, mock_get):`
		`+ """`
		`+ Test a module build with dep overrides from the branch name`
		`+ """`
		`+ cli_cmd = [`
		`+ 'rpkg',`
		`+ '--path', self.cloned_repo_path,`
		`+ 'module-build',`
		`+ 'git://pkgs.fedoraproject.org/modules/testmodule?#79d87a5a',`
		`+ '10-fedora-27.0.1'`
		`+ ]`
		`+ mock_get.return_value.ok = True`
		`+ mock_get.return_value.json.return_value = {`
		`+ 'auth_method': 'oidc',`
		`+ 'api_version': 2`
		`+ }`
		`+ mock_oidc_req.return_value.json.return_value = [{'id': 1094}]`
		`+`
		`+ with patch('sys.argv', new=cli_cmd):`
		`+ cli = self.new_cli()`
		`+ cli.config.set('rpkg.mbs', 'base_module', 'platform')`
		`+ cli.config.set('rpkg.mbs', 'base_module_stream_regex_from_branch',`
		`+ r'(f)(?:edora)(?:\-)(\d+\.\d+\.\d+)$')`
		`+ cli.module_build()`
		`+`
		`+ exp_json = {`
		`+ 'scmurl': ('git://pkgs.fedoraproject.org/modules/testmodule?'`
		`+ '#79d87a5a'),`
		`+ 'branch': '10-fedora-27.0.1',`
		`+ 'buildrequire_overrides': {'platform': ['f27.0.1']}`
		`+ }`
		`+ exp_url = ('https://mbs.fedoraproject.org/module-build-service/2/'`
		`+ 'module-builds/')`
		`+ mock_oidc_req.assert_called_once_with(`
		`+ exp_url,`
		`+ http_method='POST',`
		`+ json=exp_json,`
		`+ scopes=self.scopes,`
		`+ timeout=120)`
		`+`
		`+ @patch('requests.get')`
		`+ @patch('openidc_client.OpenIDCClient.send_request')`
		`+ def test_module_build_override_from_branch_with_manual_input(`
		`+ self, mock_oidc_req, mock_get):`
		`+ """`
		`+ Test that dep overrides aren't preferred over manual input`
		`+ """`
		`+ cli_cmd = [`
		`+ 'rpkg',`
		`+ '--path', self.cloned_repo_path,`
		`+ 'module-build',`
		`+ 'git://pkgs.fedoraproject.org/modules/testmodule?#79d87a5a',`
		`+ '10-fedora-27.0.1',`
		`+ '--buildrequire', 'platform:f29'`
		`+ ]`
		`+ mock_get.return_value.ok = True`
		`+ mock_get.return_value.json.return_value = {`
		`+ 'auth_method': 'oidc',`
		`+ 'api_version': 2`
		`+ }`
		`+ mock_oidc_req.return_value.json.return_value = [{'id': 1094}]`
		`+`
		`+ with patch('sys.argv', new=cli_cmd):`
		`+ cli = self.new_cli()`
		`+ cli.config.set('rpkg.mbs', 'base_module', 'platform')`
		`+ cli.config.set('rpkg.mbs', 'base_module_stream_regex_from_branch',`
		`+ r'(f)(?:edora)(?:\-)(\d+\.\d+\.\d+)$')`
		`+ cli.module_build()`
		`+`
		`+ exp_json = {`
		`+ 'scmurl': ('git://pkgs.fedoraproject.org/modules/testmodule?'`
		`+ '#79d87a5a'),`
		`+ 'branch': '10-fedora-27.0.1',`
		`+ 'buildrequire_overrides': {'platform': ['f29']}`
		`+ }`
		`+ exp_url = ('https://mbs.fedoraproject.org/module-build-service/2/'`
		`+ 'module-builds/')`
		`+ mock_oidc_req.assert_called_once_with(`
		`+ exp_url,`
		`+ http_method='POST',`
		`+ json=exp_json,`
		`+ scopes=self.scopes,`
		`+ timeout=120)`
		`+`
		`+ @patch('requests.get')`
		`def test_module_build_conflicting_keys(self, mock_get):`
		`"""`
		`Test a module build with optional arguments the conflict with other arguments`

mprahl commented 5 years ago

Addresses the final part of FACTORY-3084 and relies on #389. Only the last commit is relevant to this PR.

mprahl commented 5 years ago

@cqi @jkaluza @ralph could you please review?

ralph commented on line 25 of pyrpkg/cli.py 5 years ago

Can you put an example regex in a comment here? It would help a code reader understand how ''.join(branch_search.groups()) down below.

ralph commented 5 years ago

LGTM.

2 new commits added

Add config options to parse the base module (e.g. platform) stream from the dist-git branch and apply a buildrequire override
Add the ability to pass in buildrequire and require overrides on a module build

5 years ago

mprahl commented 5 years ago

@ralph thanks for the review. I added the comment as requested.

mprahl commented 5 years ago

@lsedlar could you please review this as well?

lsedlar commented 5 years ago

Generally looks good to me. :thumbsup:

The regex handling currently allows to remove parts from the branch string, but not add anything. As far as I can tell, that is sufficient now. Do you foresee any change in the requirements?

rebased onto 89e34eb

5 years ago

rebased onto 80a3d1f

5 years ago