PR#293: Change default rpmlint configuration file - rpkg

rpkg

#293 Change default rpmlint configuration file

Merged 6 years ago by cqi. Opened 6 years ago by athoscr.

athoscr/rpkg change-default-lint-config into master

Change default rpmlint configuration file

Athos Ribeiro • 6 years ago

3cdbbb7

pyrpkg/__init__.py

file modified

+7 -1

		`@@ -2184,12 +2184,18 @@`
		`if not rpms:`
		`log.warning('No rpm found')`
		`cmd = ['rpmlint']`
		`+ default_rpmlintconf = '{0}.rpmlintrc'.format(self.module_name)`
		`if info:`
		`cmd.extend(['-i'])`
		`if rpmlintconf:`
		`cmd.extend(["-f", os.path.join(self.path, rpmlintconf)])`
		`- elif os.path.exists(os.path.join(self.path, ".rpmlint")):`
		`+ elif os.path.isfile(os.path.join(self.path, default_rpmlintconf)):`
		`+ cmd.extend(["-f", os.path.join(self.path, default_rpmlintconf)])`
		`+ elif os.path.isfile(os.path.join(self.path, ".rpmlint")):`
		`cmd.extend(["-f", os.path.join(self.path, ".rpmlint")])`
		`+ self.log.warning('.rpmlint file usage as default rpmlint configuration is deprecated '`
		`+ 'and will be removed in future version. '`
		`+ 'Use {0} instead'.format(default_rpmlintconf))`
		`cmd.append(os.path.join(self.path, self.spec))`
		`if os.path.exists(os.path.join(self.path, srpm)):`
		`cmd.append(os.path.join(self.path, srpm))`

pyrpkg/cli.py

file modified

+2 -2

		`@@ -625,8 +625,8 @@`
		`'lint', help='Run rpmlint against local spec and build output if '`
		`'present.',`
		`description='Rpmlint can be configured using the --rpmlintconf/-r'`
		`- ' option or by setting a .rpmlint file in the '`
		`- 'working directory')`
		`+ ' option or by setting a <pkgname>.rpmlintrc file in '`
		`+ 'the working directory')`
		`lint_parser.add_argument(`
		`'--info', '-i', default=False, action='store_true',`
		`help='Display explanations for reported messages')`

tests/test_cli.py

file modified

+54

		`@@ -886,6 +886,60 @@`
		`rpmlint = ['rpmlint', '-i', os.path.join(cli.cmd.path, cli.cmd.spec)]`
		`_run_command.assert_called_once_with(rpmlint, shell=True)`

		`+ @patch('pyrpkg.Commands._run_command')`
		`+ def test_lint_with_default_config_file(self, _run_command):`
		`+ self.checkout_branch(git.Repo(self.cloned_repo_path), 'eng-rhel-7')`
		`+`
		`+ lint_config_path = os.path.join(self.cloned_repo_path, 'docpkg.rpmlintrc')`
		`+ open(lint_config_path, 'a').close()`
		`+`
		`+ cli_cmd = ['rpkg', '--module-name', 'docpkg', '--path', self.cloned_repo_path, 'lint']`
		`+`
		`+ with patch('sys.argv', new=cli_cmd):`
		`+ cli = self.new_cli()`
		`+ cli.lint()`
		`+`
		`+ rpmlint = ['rpmlint', '-f', lint_config_path, os.path.join(cli.cmd.path, cli.cmd.spec)]`
		`+ _run_command.assert_called_once_with(rpmlint, shell=True)`
		`+`
		`+ @patch('pyrpkg.Commands._run_command')`
		`+ def test_lint_with_default_and_deprecated_config_files(self, _run_command):`
		`+ self.checkout_branch(git.Repo(self.cloned_repo_path), 'eng-rhel-7')`
		`+`
		`+ lint_config_path = os.path.join(self.cloned_repo_path, 'docpkg.rpmlintrc')`
		`+ open(lint_config_path, 'a').close()`
		`+ deprecated_lint_config_path = os.path.join(self.cloned_repo_path, '.rpmlint')`
		`+ open(deprecated_lint_config_path, 'a').close()`
		`+`
		`+ cli_cmd = ['rpkg', '--module-name', 'docpkg', '--path', self.cloned_repo_path, 'lint']`
		`+`
		`+ with patch('sys.argv', new=cli_cmd):`
		`+ cli = self.new_cli()`
		`+ cli.lint()`
		`+`
		`+ rpmlint = ['rpmlint', '-f', lint_config_path, os.path.join(cli.cmd.path, cli.cmd.spec)]`
		`+ _run_command.assert_called_once_with(rpmlint, shell=True)`
		`+`
		`+ @patch('pyrpkg.Commands._run_command')`
		`+ def test_lint_with_custom_config_file(self, _run_command):`
		`+ self.checkout_branch(git.Repo(self.cloned_repo_path), 'eng-rhel-7')`
		`+`
		`+ lint_config_path = os.path.join(self.cloned_repo_path, 'docpkg.rpmlintrc')`
		`+ open(lint_config_path, 'a').close()`
		`+ custom_lint_config_path = os.path.join(self.cloned_repo_path, 'custom.rpmlint')`
		`+ open(custom_lint_config_path, 'a').close()`
		`+`
		`+ cli_cmd = ['rpkg', '--module-name', 'docpkg', '--path', self.cloned_repo_path,`
		`+ 'lint', '--rpmlintconf', 'custom.rpmlint']`
		`+`
		`+ with patch('sys.argv', new=cli_cmd):`
		`+ cli = self.new_cli()`
		`+ cli.lint()`
		`+`
		`+ rpmlint = ['rpmlint', '-f', custom_lint_config_path, os.path.join(cli.cmd.path,`
		`+ cli.cmd.spec)]`
		`+ _run_command.assert_called_once_with(rpmlint, shell=True)`
		`+`

		`class TestGitUrl(CliTestCase):`

athoscr commented 6 years ago

Current rpmlint configuration file (.rpmlint) is hidden. Making the file
visible may encourage packagers looking at other repositories to
properly configure rpmlint runs and omit false positives. This commit
makes <module_name>.rpmlintrc the default configuration file and
triggers a deprecation message whenever a .rpmlint file is implicitly
used.

Signed-off-by: Athos Ribeiro athoscr@fedoraproject.org

ngompa commented 6 years ago

:thumbsup:

rebased onto a823b8d1164b1ea18ac04905dc8a137e6dae302e

6 years ago

ralph commented 6 years ago

Good idea.

athoscr commented 6 years ago

The idea came up in a discussion in fedora-devel mailing lists (I had no part in the discussion, but I did like the idea).

kparal commented 6 years ago

I haven't found any place where this config file is mentioned (except for fedpkg man page, which is not linkable), so I went ahead and added a note at https://fedoraproject.org/wiki/Package_maintenance_guide (next to fedpkg lint). I used this new name already.

Also please note we added support for the same config name in task-rpmlint (shown in bodhi):
https://pagure.io/taskotron/task-rpmlint/issue/5

cqi commented on line 5 of pyrpkg/__init__.py 6 years ago

Please use property self.module_name.

cqi commented 6 years ago

Please also add a test for the new default rpmlintconf. There is already a test case TestLint containing some tests for lint command. The new test could be added to that test case class.

rebased onto b00462405174e789f01b675c4023948dadda2dde

6 years ago

rebased onto 5a8cac41b2cb588fd4c83837c4446b13f4449937

6 years ago

athoscr commented 6 years ago

I addressed the requested points and changed some documentation I missed in the first patch.

cqi commented on line 39 of tests/test_cli.py 6 years ago

This should be the problem detected by Jenkins job.

athoscr commented 6 years ago

I forgot checking the test file after adding the last tests. Fixed.

rebased onto 3cdbbb7

6 years ago