copr / copr

Clone

#1436 rpmbuild: provide the "dynamic" %bid tag
Merged 4 years ago by praiskup. Opened 4 years ago by praiskup.
Unknown source bid-tag  into  master

file modified
+25 -1 
@@ -2,6 +2,7 @@
 

  

  HELLO=https://pagure.io/copr/copr-test-sources/raw/master/f/hello-2.8-1.src.rpm
 

  EVIL_HELLO=https://pagure.io/copr/copr-test-sources/raw/master/f/evilhello-2.8-2.src.rpm
 

+ SRPM_BUILDTAG=https://pagure.io/copr/copr-test-sources/raw/master/f/buildtag-0-0.src.rpm
 

  COPR_HELLO_GIT=https://pagure.io/copr/copr-hello.git
 

  COPR_HELLO_2_GIT=https://pagure.io/copr/copr-hello-2.git
 

  DNF_COPR_ID=tested-copr
 
@@ -35,7 +36,30 @@
 

      fi
 

  }
 

  

+ # cleanProject [PROJECTNAME]
 

+ # --------------------------
 

+ # When called without PROJECTNAME, project with name $PROJECT is deleted.
 

  cleanProject()
 

  {
 

-     cleanAction copr-cli delete "$1"
 

+     _to_delete=$1
 

+     test $# -eq 0 && _to_delete=$PROJECT
 

+     cleanAction copr-cli delete "$_to_delete"
 

+ }
 

+ 

+ workdirSetup()
 

+ {
 

+     rlRun "WORKDIR=\$(mktemp -d)" 0 'Creating working directory'
 

+     rlRun "pushd $WORKDIR"
 

+ }
 

+ 

+ workdirCleanup()
 

+ {
 

+     rlRun 'popd'
 

+     rlRun "rm -r \$WORKDIR" 0 "Removing working directory"
 

+ }
 

+ 

+ setupProjectName()
 

+ {
 

+     _name=$NAME_PREFIX-$1
 

+     rlRun "PROJECT=$_name" 0 "Setting project name to $_name"
 

  }

file added
+52 
@@ -0,0 +1,52 @@
 

+ #! /bin/bash
 

+ #
 

+ # Copyright (c) 2020 Red Hat, Inc.
 

+ #
 

+ # This program is free software: you can redistribute it and/or
 

+ # modify it under the terms of the GNU General Public License as
 

+ # published by the Free Software Foundation, either version 2 of
 

+ # the License, or (at your option) any later version.
 

+ #
 

+ # This program is distributed in the hope that it will be
 

+ # useful, but WITHOUT ANY WARRANTY; without even the implied
 

+ # warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
 

+ # PURPOSE.  See the GNU General Public License for more details.
 

+ #
 

+ # You should have received a copy of the GNU General Public License
 

+ # along with this program. If not, see http://www.gnu.org/licenses/.
 

+ 

+ 

+ # Include Beaker environment
 

+ . /usr/bin/rhts-environment.sh || exit 1
 

+ . /usr/share/beakerlib/beakerlib.sh || exit 1
 

+ 

+ # Load config settings
 

+ HERE=$(dirname "$(realpath "$0")")
 

+ source "$HERE/config"
 

+ source "$HERE/helpers"
 

+ 

+ 

+ rlJournalStart
 

+     rlPhaseStartSetup
 

+         rlAssertRpm "copr-cli"
 

+         rlAssertRpm "jq"
 

+         rlAssertExists ~/.config/copr
 

+         workdirSetup
 

+         setupProjectName "buildtag"
 

+     rlPhaseEnd
 

+ 

+     rlPhaseStartTest
 

+         rlRun "copr-cli create --chroot $CHROOT $PROJECT"
 

+         rlRun -s "copr-cli build $PROJECT $SRPM_BUILDTAG --nowait"
 

+         rlRun parse_build_id
 

+         rlRun "copr watch-build $BUILD_ID"
 

+         rlRun "copr download-build $BUILD_ID"
 

+         rlRun "test -f $CHROOT/buildtag-0-0.fc$FEDORA_VERSION.copr$BUILD_ID.src.rpm"
 

+     rlPhaseEnd
 

+ 

+     rlPhaseStartCleanup
 

+         cleanProject
 

+         workdirCleanup
 

+     rlPhaseEnd
 

+ rlJournalPrintText
 

+ rlJournalEnd

file modified
+4 -1 
@@ -1,3 +1,4 @@
 

+ import re
 

  import os
 

  import sys
 

  import logging
 
@@ -14,6 +15,7 @@
 

  class MockBuilder(object):
 

      def __init__(self, task, sourcedir, resultdir, config):
 

          self.task_id = task.get("task_id")
 

+         self.build_id = re.sub("-.*", "", self.task_id)

I do not understand why this line is needed at all.

 
          self.chroot = task.get("chroot")
 

          self.buildroot_pkgs = task.get("buildroot_pkgs")
 

          self.enable_net = task.get("enable_net")
 
@@ -66,7 +68,8 @@
 

                                 enable_net=self.enable_net, use_bootstrap_container=self.use_bootstrap_container,
 

                                 repos=self.repos,
 

                                 copr_username=self.copr_username, copr_projectname=self.copr_projectname,
 

-                                modules=self.enable_modules)
 

+                                modules=self.enable_modules,
 

+                                copr_build_id=self.build_id)
 

  

      def produce_srpm(self, spec, sources, resultdir):
 

          cmd = MOCK_CALL + [

file modified
+3 
@@ -14,6 +14,9 @@
 

  

  config_opts['macros']['%copr_username'] = '{{ copr_username }}'
 

  config_opts['macros']['%copr_projectname'] = '{{ copr_projectname }}'
 

+ # Build-system's (or build) ID
 

+ config_opts['macros']['%buildtag'] = '.copr{{ copr_build_id }}'
 

+ 

  config_opts['use_bootstrap'] = {{ 'True' if use_bootstrap_container else 'False' }}
 

  

  {% if use_bootstrap_container %}

file modified
+3 
@@ -150,6 +150,9 @@
 

  

  config_opts['macros']['%copr_username'] = '@copr'
 

  config_opts['macros']['%copr_projectname'] = 'copr-dev'
 

+ # Build-system's (or build) ID
 

+ config_opts['macros']['%buildtag'] = '.copr10'
 

+ 

  config_opts['use_bootstrap'] = False

The %bid macro can be used similarly to %dist, as post-release flag.
While the %dist tag is specified by the mock build-chroot, %bid tag is
specified by the wrapping build system. Example of use is:

Release: 0%{?dist}%{?bid}

This could solve the "release autobump" problem. The use of this macro
is not at the time of committing this mandatory, nor suggested. This is
mostly for R&D, how it serves its purpose.

This commit is reaction on discussion on Fedora devel:
https://lists.fedoraproject.org/archives/list/\
devel@lists.fedoraproject.org/thread/TXLF22CSUVUIQBVHH2NEFF4IOIFHS5WK/

When the %bid tag is in use, simple re-build (resubmit button in copr
UI) of the package causes that the Release is always bumped.

Except that dnf/yum should newly always prefer the latest build of the
affected package in such situation, it also guarantees that our build
garbage collector (prunerepo) really keeps only the latest available
builds of each package (so far, when there are two exactly the same
NVRs, it is not defined which one of those is to be deleted).

Fixes: #840

We are setting the %bid macro value here, but we are not testing if it's gonna have any effect. Can we easily do that?

I am also a bit confused because the value of self.task_id is going to be something like "10-fedora-rawhide-x86_64" hence into the self.build_id will be set just 123. How we will get to a value like ".copr10"

How we will get to a value like ".copr10"

In the j2 template.

We are setting the %bid macro value here, but we are not testing if it's gonna have any effect. Can we easily do that?

We could write beaker test for that. Not sure we should require it here, I can write one
if it is your preference (such things would be better tested in mock).

In the j2 template.

Ah, sry. I missed that. In that case, the code looks good to me, +1.

IIRC there was some discussion in the past whether to have a Copr-specific macro like this and apparently we didn't implement it. I am not sure for what reason though. Or maybe we did and then (unintentionally) dropped it during some copr-rpmbuild rework, right? Yea, I think that should be it.

However, from me, this is +1 because even though I think generally people shouldn't use it, it may be significant improvement for somebody's use-case.

IIRC there was some discussion in the past whether to have a Copr-specific macro like this and apparently we didn't implement it. I am not sure for what reason though. Or maybe we did and then (unintentionally) dropped it during some copr-rpmbuild rework, right? Yea, I think that should be it.

Yes, we dropped the %copr_* macros, and then we re-added them.

However, from me, this is +1 because even though I think generally people shouldn't use it, it may be a significant improvement for somebody's use-case.

Well, the %copr_ macros aren't that useful IMO -- those can be mostly used only to
detect Copr build environment (for package build-time conditionals). Ideally, we
shouldn't have a need to detect where we build, it shouldn't matter.

I think %bid (or if anyone proposes a better macro name) could be useful in
general, and Koji could define it one day (if we found it useful). At least it is
meant to be used unconditionally with %{?bid}, so it will be ignored if not
defined.
Edited 4 years ago by praiskup

Please give the community some time (say two weeks or so) for commenting this before we merge.

%bid (or if anyone proposes a better macro name)

My original proposal was "buildtag". Didn't you like that?

"bid" is bad because it's an existing English word that means something entirely different.

I do not understand why this line is needed at all.

My original proposal was "buildtag". Didn't you like that?

Thanks for the comment. I haven't seen your proposal, can you please link that? My first thought is that the shorter name the better.

self.build_id = re.sub("-.*", "", self.task_id)
I do not understand why this line is needed at all.

Task id looks like 1231231-fedora-rawhide-x86_64.
Edited 4 years ago by praiskup

Replying by email doesn't seem to work. I'm trying again through the web interface.

My original proposal was "buildtag". Didn't you like that?

Thanks for the comment. I haven't seen your proposal, can you please link that?

My proposal is here:
https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/message/SBXYOUL6MYJ7D4TBULMI4YXYRMO3QFN5/

It definitely looks like you made this pull request as a direct
reaction to that discussion:
https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/message/P3FXVGLXH7H7Y6OHIZEUDUI246YKF7BC/

My first thought is that the shorter name the better.

This isn't code golf. Readable code takes priority over minimizing
keystrokes.

Ah, sorry. I didn't pay too much attention to that %(date) proposal, thank you
for the link. We both are reacting to the thread, indeed.

So, can we use this comment for voting -- thumb-up for %buildtag, thumb down
for %bid?

Bjorn, any objections against the baked build-id into %buildtag?

Pavel Raiskup wrote:

Bjorn, any objections against the baked build-id into %buildtag?=20

I'm not strongly attached to timestamps. A build ID should also work,
if COPR guarantees that a later build will always get a greater ID than
an earlier one.

Of course COPR build IDs and Koji task IDs won't be synchronized, but
that's not necessarily a problem. Packages aren't supposed to jump back
and forth between COPR and Fedora anyway. It may even be desirable to
have builtags from Koji always compare as greater than buildtags from
COPR.

As I'm overvoted now, switching to %buildtag. Thanks!

Just my concern, when speaking about %dist, we call it "dist tag".
OTOH, the %buildtag has the "tag" in the macro name, which sounds
a bit unnecessary to me (we also don't have %disttag).

Pavel Raiskup wrote:

the %buildtag has the "tag" in the macro name

I considered calling it "build", but naming a macro the same as a spec
file section seemed like a decidedly bad idea, if it would even be
allowed.

rebased onto 15741b3f2689b180386ff8aa4586f11305677915
4 years ago

Updated according to Björn's suggestion, and test added.

[copr-build]

I considered calling it "build"

I tried that as well ... basically btag, buildid, buildtag, bid, build,
bt, and several others. And I liked bid the most (I also, as a small
joke, liked the meaning of the English word "bid", like build-system bid).

Nevermind, we can define another variant in the future, and keep defining
this one as a backward compatibility macro. %buildtag tag wins now.

rebased onto 44737c4
4 years ago

Pull-Request has been merged by praiskup
4 years ago
Changes Summary 5
+25 -1
file changed
beaker-tests/Sanity/copr-cli-basic-operations/helpers
+52
file added
beaker-tests/Sanity/copr-cli-basic-operations/runtest-buildtag.sh
+4 -1
file changed
rpmbuild/copr_rpmbuild/builders/mock.py
+3 -0
file changed
rpmbuild/mock.cfg.j2
+3 -0
file changed
rpmbuild/tests/test_mock.py
Powered by Pagure 5.14.1
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.