PR#448: create copr-rpmbuild-all subpackage - copr/copr

		`@@ -25,15 +25,15 @@`
		`# rpkg spec --sources`
		`Source0: {{{ git_dir_archive }}}`

		`- BuildRequires: %python-devel`
		`- BuildRequires: %rpm_python`
		`+ BuildRequires: %{python}-devel`
		`+ BuildRequires: %{rpm_python}`
		`BuildRequires: asciidoc`
		`- BuildRequires: %python-setuptools`
		`- BuildRequires: %python-pytest`
		`- BuildRequires: %python_pfx-munch`
		`- BuildRequires: %python-requests`
		`- BuildRequires: %python_pfx-jinja2`
		`- BuildRequires: %python-configparser`
		`+ BuildRequires: %{python}-setuptools`
		`+ BuildRequires: %{python}-pytest`
		`+ BuildRequires: %{python_pfx}-munch`
		`+ BuildRequires: %{python}-requests`
		`+ BuildRequires: %{python_pfx}-jinja2`
		`+ BuildRequires: %{python}-configparser`

		`BuildRequires: python-rpm-macros`

		`@@ -42,20 +42,21 @@`
		`%endif`

		`Requires: %python`
		`- Requires: %python_pfx-jinja2`
		`- Requires: %python_pfx-munch`
		`- Requires: %python-configparser`
		`- Requires: %python-requests`
		`- Requires: %python_pfx-simplejson`
		`+ Requires: %{python_pfx}-jinja2`
		`+ Requires: %{python_pfx}-munch`
		`+ Requires: %{python}-configparser`
		`+ Requires: %{python}-requests`
		`+ Requires: %{python_pfx}-simplejson`

		`Requires: mock`
		`Requires: git`
		`Requires: git-svn`
		`- Requires: rpkg`
		`Requires: expect`
		`- Requires: tito`

		`%if 0%{?fedora} \|\| 0%{?rhel} > 7`
		`+ Recommends: rpkg`
		`+ Recommends: python-srpm-macros`
		`+ Suggests: tito`
		`Suggests: rubygem-gem2rpm`
		`Suggests: pyp2rpm`
		`%endif`
		`@@ -65,6 +66,22 @@`
		`Example: copr-rpmbuild 12345-epel-7-x86_64 will locally`
		`build build-id 12345 for chroot epel-7-x86_64.`

		`+ %package -n copr-builder`
		`+ Summary: copr-rpmbuild with all weak dependencies`
		`+ Requires: %{name} = %{version}-%{release}`
		`+ Requires: rpkg`
		`+ Requires: tito`
		`+ Requires: rubygem-gem2rpm`
		`+ Requires: pyp2rpm`
		`+ Requires: python-srpm-macros`
		`+`
		`+ %description -n copr-builder`
		`+ Provides command capable of running COPR build-tasks.`
		`+ Example: copr-rpmbuild 12345-epel-7-x86_64 will locally`
		`+ build build-id 12345 for chroot epel-7-x86_64.`
		`+`
		`+ This package contains all optional modules for building SRPM.`
		`+`
		`%prep`
		`%setup -q`

		`@@ -113,6 +130,9 @@`
		`%config(noreplace) %{_sysconfdir}/copr-rpmbuild/rpkg.conf.j2`
		`%config(noreplace) %{_sysconfdir}/copr-rpmbuild/make_srpm_mock.cfg`

		`+ %files -n copr-builder`
		`+ %license LICENSE`
		`+`
		`%changelog`
		`{{{ git_dir_changelog since_tag=copr-rpmbuild-0.18-1 }}}`

rpmbuild/copr_rpmbuild/helpers.py

file modified

+5 -2

		`@@ -56,10 +56,13 @@`
		`:returns munch.Munch(cmd, stdout, stderr, returncode)`
		`"""`
		`log.info('Running: ' + cmd_readable(cmd))`
		`- process = subprocess.Popen(`
		`- cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, cwd=cwd, preexec_fn=preexec_fn)`
		`+`
		`try:`
		`+ process = subprocess.Popen(`
		`+ cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, cwd=cwd, preexec_fn=preexec_fn)`
		`(stdout, stderr) = process.communicate()`
		`+ except FileNotFoundError:`
		+ raise RuntimeError("Package with `{}` command is not installed".format(cmd[0]))
		`except OSError as e:`
		`raise RuntimeError(str(e))`

msuchy commented 5 years ago

Addressing
https://pagure.io/copr/copr/issue/437

praiskup commented 5 years ago

IMO this solves non-existing issue, in no way this solves the copr mock-config problem. Btw., didn't you want to move rpkg and tito Requires, too?

What about re-using copr-builder name, instead of copr-rpmbuild-all which says nothing about the semantics of the package.

frostyx commented 5 years ago

IMO this solves non-existing issue, in no way this solves the copr mock-config problem.

Regardless copr mock-config, I think that this is a good idea. The issue is that there are efforts for both having a copr-rpmbuild a client tool and also a package for preparing a builder, which are going against themselves. Having the base functionality in copr-rpmbuild package and meta package copr-rpmbuild-all (or full, meta, etc) would be a nice compromise.

What about re-using copr-builder name, instead of copr-rpmbuild-all which says nothing about the semantics of the package.

I like this idea, but I wouldn't rename the current copr-rpmbuild to anything. It would be copr-rpmbuild with base functionality and copr-builder meta package.

clime commented 5 years ago

+1 for copr-builder providing this.

praiskup commented 5 years ago

So, just to assure you -> feel free to re-use ./copr/copr-builder.spec for this purpose. That package
- used to have that semantics
- provides script which nobody uses nowadays, that can be dropped
- that package is in Fedora, so no review would be needed... just an update

I'm repeating myself, but I don't really like complication in copr-rpmbuild code in any way, so I don't like the fact it plays the "client tool" role -- it means we have to pay attention to its API too much. I feel in my linux bones that - if there is some interesting and re-usable part of copr-rpmbuild - it should be cut-out into separate script (and maybe package/project) so others can use it easily, without depending on too much stuff brought by whole copr-rpmbuild. Btw., there's still is the copr-* prefix, so any use-case outside of Copr scope sounds unwise.