[libvirt] [jenkins-ci PATCH 0/5] Drop last use of ~/rpmbuild, other cleanups
Patch 1/5 ensures ~/rpmbuild is no longer used, while the remaining ones increase consistency in the various rules used to build RPMs. Andrea Bolognani (5): jobs: Call rpmbuild explicitly for Python projects jobs: Call rpmbuild and sed consistently jobs: Introduce strip_buildrequires jobs: Minimize strip_buildrequires jobs: Don't remove non-existing archives jobs/autotools.yaml | 7 ++----- jobs/defaults.yaml | 5 +++++ jobs/perl-modulebuild.yaml | 5 +---- jobs/python-distutils.yaml | 5 +++-- projects/osinfo-db.yaml | 2 +- 5 files changed, 12 insertions(+), 12 deletions(-) -- 2.17.1
Instead of using the custom 'rpm' target of setup.py, generate a dist archive using the 'sdist' target and then call rpmbuild ourselves: this way we can define _topdir and stop artifacts from ending up in ~/rpmbuild. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- jobs/python-distutils.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/jobs/python-distutils.yaml b/jobs/python-distutils.yaml index 0b20b17..5601d26 100644 --- a/jobs/python-distutils.yaml +++ b/jobs/python-distutils.yaml @@ -122,7 +122,9 @@ {global_env} {local_env} sed -i -e 's/BuildRequires: libvirt.*devel.*//' *.spec.in - $PYTHON ./setup.py rpm + rm -f dist/*.tar.{{ archive_format }} + $PYTHON ./setup.py sdist + rpmbuild --clean --define "_topdir `pwd`/rpmbuild" -ta dist/*.tar.{{ archive_format }} publishers: - email: recipients: '{obj:spam}' -- 2.17.1
On Fri, Aug 10, 2018 at 03:37:41PM +0200, Andrea Bolognani wrote:
Instead of using the custom 'rpm' target of setup.py, generate a dist archive using the 'sdist' target and then call rpmbuild ourselves: this way we can define _topdir and stop artifacts from ending up in ~/rpmbuild.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- jobs/python-distutils.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/jobs/python-distutils.yaml b/jobs/python-distutils.yaml index 0b20b17..5601d26 100644 --- a/jobs/python-distutils.yaml +++ b/jobs/python-distutils.yaml @@ -122,7 +122,9 @@ {global_env} {local_env} sed -i -e 's/BuildRequires: libvirt.*devel.*//' *.spec.in - $PYTHON ./setup.py rpm + rm -f dist/*.tar.{{ archive_format }} + $PYTHON ./setup.py sdist + rpmbuild --clean --define "_topdir `pwd`/rpmbuild" -ta dist/*.tar.{{ archive_format }}
So what if you used a standard bdist_rpm command from distutils core, I believe $PYTHON ./setup.py bdist_rpm --bdist-base <foo> would be equal to your _topdir. Although, that's just what I've digested from distutils docs, so even though bdist_rpm has a plethora of options you can specify there can always be one we'll be missing :P Erik
publishers: - email: recipients: '{obj:spam}' -- 2.17.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, 2018-08-20 at 16:01 +0200, Erik Skultety wrote:
On Fri, Aug 10, 2018 at 03:37:41PM +0200, Andrea Bolognani wrote:
- $PYTHON ./setup.py rpm + rm -f dist/*.tar.{{ archive_format }} + $PYTHON ./setup.py sdist + rpmbuild --clean --define "_topdir `pwd`/rpmbuild" -ta dist/*.tar.{{ archive_format }}
So what if you used a standard bdist_rpm command from distutils core, I believe $PYTHON ./setup.py bdist_rpm --bdist-base <foo> would be equal to your _topdir. Although, that's just what I've digested from distutils docs, so even though bdist_rpm has a plethora of options you can specify there can always be one we'll be missing :P
I haven't been able to find any bdist_rpm documentation that is not filed under Python 2, which leads me to believe it might not be as supported (if at all) under Python 3; moreover, the current documentation[1] seems to point to FPM as the preferred way to generate RPM packages, but that process doesn't looks like it involves spec files at all and bundle a whole lot of other stuff along with your actual software, so I'd say it's not really suitable for our purpose. In any case, I would still prefer the two-step approach (dist plus rpmbuild) to building RPMs because it is consistent with what we do for all other build systems (autotools and Perl's Module::Build). [1] https://packaging.python.org/overview/#operating-system-packages -- Andrea Bolognani / Red Hat / Virtualization
On Mon, Aug 20, 2018 at 04:37:13PM +0200, Andrea Bolognani wrote:
On Mon, 2018-08-20 at 16:01 +0200, Erik Skultety wrote:
On Fri, Aug 10, 2018 at 03:37:41PM +0200, Andrea Bolognani wrote:
- $PYTHON ./setup.py rpm + rm -f dist/*.tar.{{ archive_format }} + $PYTHON ./setup.py sdist + rpmbuild --clean --define "_topdir `pwd`/rpmbuild" -ta dist/*.tar.{{ archive_format }}
So what if you used a standard bdist_rpm command from distutils core, I believe $PYTHON ./setup.py bdist_rpm --bdist-base <foo> would be equal to your _topdir. Although, that's just what I've digested from distutils docs, so even though bdist_rpm has a plethora of options you can specify there can always be one we'll be missing :P
I haven't been able to find any bdist_rpm documentation that is not filed under Python 2, which leads me to believe it might not be as supported (if at all) under Python 3; moreover, the current documentation[1] seems to point to FPM as the preferred way to generate RPM packages, but that process doesn't looks like it involves spec files at all and bundle a whole lot of other stuff along with your actual software, so I'd say it's not really suitable for our purpose.
In any case, I would still prefer the two-step approach (dist plus rpmbuild) to building RPMs because it is consistent with what we do for all other build systems (autotools and Perl's Module::Build).
[1] https://packaging.python.org/overview/#operating-system-packages
Fair enough, Reviewed-by: Erik Skultety <eskultet@redhat.com>
Doing so will allow us to refactor away the common parts. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- jobs/autotools.yaml | 8 ++++---- jobs/perl-modulebuild.yaml | 6 +++--- jobs/python-distutils.yaml | 2 +- projects/osinfo-db.yaml | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/jobs/autotools.yaml b/jobs/autotools.yaml index f1ebf7b..ebf5bd8 100644 --- a/jobs/autotools.yaml +++ b/jobs/autotools.yaml @@ -167,12 +167,12 @@ {global_env} {local_env} cd build - sed -i -e 's/BuildRequires: osinfo-db.*//' {name}.spec - sed -i -e 's/BuildRequires: libvirt.*devel.*//' {name}.spec - sed -i -e 's/BuildRequires: pkgconfig(libvirt.*).*//' {name}.spec + sed -i -e 's/BuildRequires: *osinfo-db.*//' *.spec* + sed -i -e 's/BuildRequires: *libvirt.*devel.*//' *.spec* + sed -i -e 's/BuildRequires: *pkgconfig(libvirt.*).*//' *.spec* rm -f *.tar.{archive_format} $MAKE dist - rpmbuild --clean --define "_topdir `pwd`/rpmbuild" -ta {name}-*.tar.{archive_format} + rpmbuild --clean --define "_topdir `pwd`/rpmbuild" -ta *.tar.{archive_format} publishers: - email: recipients: '{obj:spam}' diff --git a/jobs/perl-modulebuild.yaml b/jobs/perl-modulebuild.yaml index 3f3f537..93f7054 100644 --- a/jobs/perl-modulebuild.yaml +++ b/jobs/perl-modulebuild.yaml @@ -121,9 +121,9 @@ - shell: | {global_env} {local_env} - sed -i -e 's/BuildRequires: *perl-Sys-Virt.*//' *.spec - sed -i -e 's/BuildRequires: *perl(Sys::Virt).*//' *.spec - sed -i -e 's/BuildRequires: *libvirt.*devel.*//' *.spec + sed -i -e 's/BuildRequires: *perl-Sys-Virt.*//' *.spec* + sed -i -e 's/BuildRequires: *perl(Sys::Virt).*//' *.spec* + sed -i -e 's/BuildRequires: *libvirt.*devel.*//' *.spec* rm -f *.tar.{archive_format} perl Build dist rpmbuild --clean --define "_topdir `pwd`/rpmbuild" -ta *.tar.{archive_format} diff --git a/jobs/python-distutils.yaml b/jobs/python-distutils.yaml index 5601d26..2cdf25b 100644 --- a/jobs/python-distutils.yaml +++ b/jobs/python-distutils.yaml @@ -121,7 +121,7 @@ - shell: | {global_env} {local_env} - sed -i -e 's/BuildRequires: libvirt.*devel.*//' *.spec.in + sed -i -e 's/BuildRequires: *libvirt.*devel.*//' *.spec* rm -f dist/*.tar.{{ archive_format }} $PYTHON ./setup.py sdist rpmbuild --clean --define "_topdir `pwd`/rpmbuild" -ta dist/*.tar.{{ archive_format }} diff --git a/projects/osinfo-db.yaml b/projects/osinfo-db.yaml index d154c98..b8b609e 100644 --- a/projects/osinfo-db.yaml +++ b/projects/osinfo-db.yaml @@ -19,5 +19,5 @@ parent_jobs: 'osinfo-db-master-check' machines: '{rpm_machines}' command: | - perl -i -p -e 's/BuildRequires: osinfo-db-tools.*//' osinfo-db.spec + sed -i -e 's/BuildRequires: *osinfo-db-tools.*//' *.spec* rpmbuild --clean --define "_topdir `pwd`/rpmbuild" --define "_sourcedir `pwd`" -ba osinfo-db.spec -- 2.17.1
On Fri, Aug 10, 2018 at 03:37:42PM +0200, Andrea Bolognani wrote:
Doing so will allow us to refactor away the common parts.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- Reviewed-by: Erik Skultety <eskultet@redhat.com>
There's no harm in attempting to strip more BuildRequires than are present in a spec file, so define a shell snippet we can reuse for the the purpose without duplicating the same lines all over the place. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- jobs/autotools.yaml | 4 +--- jobs/defaults.yaml | 7 +++++++ jobs/perl-modulebuild.yaml | 4 +--- jobs/python-distutils.yaml | 2 +- projects/osinfo-db.yaml | 2 +- 5 files changed, 11 insertions(+), 8 deletions(-) diff --git a/jobs/autotools.yaml b/jobs/autotools.yaml index ebf5bd8..f8a7e87 100644 --- a/jobs/autotools.yaml +++ b/jobs/autotools.yaml @@ -167,9 +167,7 @@ {global_env} {local_env} cd build - sed -i -e 's/BuildRequires: *osinfo-db.*//' *.spec* - sed -i -e 's/BuildRequires: *libvirt.*devel.*//' *.spec* - sed -i -e 's/BuildRequires: *pkgconfig(libvirt.*).*//' *.spec* + {strip_buildrequires} rm -f *.tar.{archive_format} $MAKE dist rpmbuild --clean --define "_topdir `pwd`/rpmbuild" -ta *.tar.{archive_format} diff --git a/jobs/defaults.yaml b/jobs/defaults.yaml index eb04f32..e4d1f2c 100644 --- a/jobs/defaults.yaml +++ b/jobs/defaults.yaml @@ -23,6 +23,13 @@ archive_format: gz global_env: | local_env: | + strip_buildrequires: | + sed -i -e 's/BuildRequires: *libvirt.*devel.*//' *.spec* + sed -i -e 's/BuildRequires: *osinfo-db-tools.*//' *.spec* + sed -i -e 's/BuildRequires: *osinfo-db.*//' *.spec* + sed -i -e 's/BuildRequires: *perl(Sys::Virt).*//' *.spec* + sed -i -e 's/BuildRequires: *perl-Sys-Virt.*//' *.spec* + sed -i -e 's/BuildRequires: *pkgconfig(libvirt.*).*//' *.spec* mingw32_local_env: | export VIRT_PREFIX="$VIRT_PREFIX/i686-w64-mingw32/sys-root/mingw" export PKG_CONFIG_PATH="$VIRT_PREFIX/lib/pkgconfig" diff --git a/jobs/perl-modulebuild.yaml b/jobs/perl-modulebuild.yaml index 93f7054..8b30d7e 100644 --- a/jobs/perl-modulebuild.yaml +++ b/jobs/perl-modulebuild.yaml @@ -121,9 +121,7 @@ - shell: | {global_env} {local_env} - sed -i -e 's/BuildRequires: *perl-Sys-Virt.*//' *.spec* - sed -i -e 's/BuildRequires: *perl(Sys::Virt).*//' *.spec* - sed -i -e 's/BuildRequires: *libvirt.*devel.*//' *.spec* + {strip_buildrequires} rm -f *.tar.{archive_format} perl Build dist rpmbuild --clean --define "_topdir `pwd`/rpmbuild" -ta *.tar.{archive_format} diff --git a/jobs/python-distutils.yaml b/jobs/python-distutils.yaml index 2cdf25b..eae5b6f 100644 --- a/jobs/python-distutils.yaml +++ b/jobs/python-distutils.yaml @@ -121,7 +121,7 @@ - shell: | {global_env} {local_env} - sed -i -e 's/BuildRequires: *libvirt.*devel.*//' *.spec* + {strip_buildrequires} rm -f dist/*.tar.{{ archive_format }} $PYTHON ./setup.py sdist rpmbuild --clean --define "_topdir `pwd`/rpmbuild" -ta dist/*.tar.{{ archive_format }} diff --git a/projects/osinfo-db.yaml b/projects/osinfo-db.yaml index b8b609e..7ee84a3 100644 --- a/projects/osinfo-db.yaml +++ b/projects/osinfo-db.yaml @@ -19,5 +19,5 @@ parent_jobs: 'osinfo-db-master-check' machines: '{rpm_machines}' command: | - sed -i -e 's/BuildRequires: *osinfo-db-tools.*//' *.spec* + {strip_buildrequires} rpmbuild --clean --define "_topdir `pwd`/rpmbuild" --define "_sourcedir `pwd`" -ba osinfo-db.spec -- 2.17.1
On Fri, Aug 10, 2018 at 03:37:43PM +0200, Andrea Bolognani wrote:
There's no harm in attempting to strip more BuildRequires than are present in a spec file, so define a shell snippet we can reuse for the the purpose without duplicating the same lines all over the place.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- Reviewed-by: Erik Skultety <eskultet@redhat.com>
We can drop some entries and tweak some others to be less verbose without losing in functionality. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- jobs/defaults.yaml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/jobs/defaults.yaml b/jobs/defaults.yaml index e4d1f2c..bab5bc4 100644 --- a/jobs/defaults.yaml +++ b/jobs/defaults.yaml @@ -24,11 +24,9 @@ global_env: | local_env: | strip_buildrequires: | - sed -i -e 's/BuildRequires: *libvirt.*devel.*//' *.spec* - sed -i -e 's/BuildRequires: *osinfo-db-tools.*//' *.spec* + sed -i -e 's/BuildRequires: *libvirt.*//' *.spec* sed -i -e 's/BuildRequires: *osinfo-db.*//' *.spec* sed -i -e 's/BuildRequires: *perl(Sys::Virt).*//' *.spec* - sed -i -e 's/BuildRequires: *perl-Sys-Virt.*//' *.spec* sed -i -e 's/BuildRequires: *pkgconfig(libvirt.*).*//' *.spec* mingw32_local_env: | export VIRT_PREFIX="$VIRT_PREFIX/i686-w64-mingw32/sys-root/mingw" -- 2.17.1
On Fri, Aug 10, 2018 at 03:37:44PM +0200, Andrea Bolognani wrote:
We can drop some entries and tweak some others to be less verbose without losing in functionality.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- jobs/defaults.yaml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/jobs/defaults.yaml b/jobs/defaults.yaml index e4d1f2c..bab5bc4 100644 --- a/jobs/defaults.yaml +++ b/jobs/defaults.yaml @@ -24,11 +24,9 @@ global_env: | local_env: | strip_buildrequires: | - sed -i -e 's/BuildRequires: *libvirt.*devel.*//' *.spec* - sed -i -e 's/BuildRequires: *osinfo-db-tools.*//' *.spec* + sed -i -e 's/BuildRequires: *libvirt.*//' *.spec* sed -i -e 's/BuildRequires: *osinfo-db.*//' *.spec*
sed -i -e 's/BuildRequires: *perl(Sys::Virt).*//' *.spec* - sed -i -e 's/BuildRequires: *perl-Sys-Virt.*//' *.spec*
I assume because ^these 2 are somehow identical? Reviewed-by: Erik Skultety <eskultet@redhat.com>
On Mon, 2018-08-20 at 16:11 +0200, Erik Skultety wrote:
On Fri, Aug 10, 2018 at 03:37:44PM +0200, Andrea Bolognani wrote:
sed -i -e 's/BuildRequires: *perl(Sys::Virt).*//' *.spec* - sed -i -e 's/BuildRequires: *perl-Sys-Virt.*//' *.spec*
I assume because ^these 2 are somehow identical?
Well, either one can be used to express the same dependency but strictly speaking they're not identical... However, we only use the first one in practice so the second one is unnecessary at the moment - we can add it back the first time we get actual users. -- Andrea Bolognani / Red Hat / Virtualization
None of our jobs create the archives as a side-effect, so trying to remove them before generating them is pointless. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- jobs/autotools.yaml | 1 - jobs/perl-modulebuild.yaml | 1 - jobs/python-distutils.yaml | 1 - 3 files changed, 3 deletions(-) diff --git a/jobs/autotools.yaml b/jobs/autotools.yaml index f8a7e87..4e075bd 100644 --- a/jobs/autotools.yaml +++ b/jobs/autotools.yaml @@ -168,7 +168,6 @@ {local_env} cd build {strip_buildrequires} - rm -f *.tar.{archive_format} $MAKE dist rpmbuild --clean --define "_topdir `pwd`/rpmbuild" -ta *.tar.{archive_format} publishers: diff --git a/jobs/perl-modulebuild.yaml b/jobs/perl-modulebuild.yaml index 8b30d7e..855e6ba 100644 --- a/jobs/perl-modulebuild.yaml +++ b/jobs/perl-modulebuild.yaml @@ -122,7 +122,6 @@ {global_env} {local_env} {strip_buildrequires} - rm -f *.tar.{archive_format} perl Build dist rpmbuild --clean --define "_topdir `pwd`/rpmbuild" -ta *.tar.{archive_format} publishers: diff --git a/jobs/python-distutils.yaml b/jobs/python-distutils.yaml index eae5b6f..e640434 100644 --- a/jobs/python-distutils.yaml +++ b/jobs/python-distutils.yaml @@ -122,7 +122,6 @@ {global_env} {local_env} {strip_buildrequires} - rm -f dist/*.tar.{{ archive_format }} $PYTHON ./setup.py sdist rpmbuild --clean --define "_topdir `pwd`/rpmbuild" -ta dist/*.tar.{{ archive_format }} publishers: -- 2.17.1
On Fri, Aug 10, 2018 at 03:37:45PM +0200, Andrea Bolognani wrote:
None of our jobs create the archives as a side-effect, so trying to remove them before generating them is pointless.
Who cleans them after the jobs below? Is there a chance that you could trigger these jobs in a sequence that would expect the archives to be removed but wouldn't? If there is a risk that this could happen, then I think this patch should be dropped. Erik
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- jobs/autotools.yaml | 1 - jobs/perl-modulebuild.yaml | 1 - jobs/python-distutils.yaml | 1 - 3 files changed, 3 deletions(-)
diff --git a/jobs/autotools.yaml b/jobs/autotools.yaml index f8a7e87..4e075bd 100644 --- a/jobs/autotools.yaml +++ b/jobs/autotools.yaml @@ -168,7 +168,6 @@ {local_env} cd build {strip_buildrequires} - rm -f *.tar.{archive_format} $MAKE dist rpmbuild --clean --define "_topdir `pwd`/rpmbuild" -ta *.tar.{archive_format} publishers: diff --git a/jobs/perl-modulebuild.yaml b/jobs/perl-modulebuild.yaml index 8b30d7e..855e6ba 100644 --- a/jobs/perl-modulebuild.yaml +++ b/jobs/perl-modulebuild.yaml @@ -122,7 +122,6 @@ {global_env} {local_env} {strip_buildrequires} - rm -f *.tar.{archive_format} perl Build dist rpmbuild --clean --define "_topdir `pwd`/rpmbuild" -ta *.tar.{archive_format} publishers: diff --git a/jobs/python-distutils.yaml b/jobs/python-distutils.yaml index eae5b6f..e640434 100644 --- a/jobs/python-distutils.yaml +++ b/jobs/python-distutils.yaml @@ -122,7 +122,6 @@ {global_env} {local_env} {strip_buildrequires} - rm -f dist/*.tar.{{ archive_format }} $PYTHON ./setup.py sdist rpmbuild --clean --define "_topdir `pwd`/rpmbuild" -ta dist/*.tar.{{ archive_format }} publishers: -- 2.17.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, 2018-08-20 at 17:00 +0200, Erik Skultety wrote:
On Fri, Aug 10, 2018 at 03:37:45PM +0200, Andrea Bolognani wrote:
None of our jobs create the archives as a side-effect, so trying to remove them before generating them is pointless.
Who cleans them after the jobs below? Is there a chance that you could trigger these jobs in a sequence that would expect the archives to be removed but wouldn't? If there is a risk that this could happen, then I think this patch should be dropped.
Cleanup is performed every time new commits are fetched from git, so stale data shouldn't be a problem in practice; that said, I agree that there's a chance we might end up using an old archive if jobs are manually triggered, and keeping the extra rm there is not really hurting anyone, so let's drop this commit :) By the way, I just noticed that...
{strip_buildrequires} - rm -f dist/*.tar.{{ archive_format }} $PYTHON ./setup.py sdist rpmbuild --clean --define "_topdir `pwd`/rpmbuild" -ta dist/*.tar.{{ archive_format }}
... I mistakenly used the Ansible-style variable substitution instead of the Jenkins Job Builder-style one, so I fixed it up before pushing the first four patches. Thank you for the review! :) -- Andrea Bolognani / Red Hat / Virtualization
participants (2)
-
Andrea Bolognani -
Erik Skultety