#27 Add script to review package PRs
Merged 2 years ago by decathorpe. Opened 2 years ago by cipherboy.
cipherboy/stewardship-sig review-prs  into  master

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

+ #!/bin/bash

+ 

+ # An error handling + verbose execution script

+ function _e() {(

+     # Explicitly disable regular error handling

+     set +euo pipefail

+ 

+     if [ ! -z "$VERBOSE" ]; then

+         echo "[exec]" "$@" 1>&2

+     fi

+ 

+     # Get the name of the command to execute

+     local cmd="$1"

+     shift

+ 

+     # Execute it with the desired arguments

+     "$cmd" "$@"

+     local ret=$?

+ 

+     # If the command exited with a non-zero status; show it

+     if (( ret != 0 )); then

+         echo "$cmd" exited with non-zero status: $ret 1>&2

+         echo "    command line: $cmd" "$@" 1>&2

+     fi

+ 

+     # Return with the same status so we behave nicely

+     return $ret

+ )}

+ 

+ function check_copr_repo() {

+     local copr="$1"

+ 

+     # copr modify returns 0 when the COPR repo exists, and non-zero when it

+     # doesn't exist (but noisily).

+     if ! _e copr modify "$copr" >/dev/null 2>&1; then

+         # Since the repository doesn't exist, create it with:

+         #

+         # - Don't list it on the home page

+         # - Delete it after 30 days

+         # - Use the Fedora Rawhide x86_64 chroot

+         _e copr create --unlisted-on-hp on --delete-after-days 30 \

+             --chroot fedora-rawhide-x86_64 "$name"

+     fi

+ }

+ 

+ function build_package() {

+     local copr="$1"

+     local package="$2"

+ 

+     # Since we use a double colon for name, we don't need to worry about

+     # clobbering the protocol specification and can use greedy matches.

+     local name="${package%::*}"

+     local branch="${package##*::}"

+ 

+     # URL we'll eventually clone.

+     local url=""

+ 

+     # If there's a double forward slash (//) in the package name, we assume

+     # that it is the URL form, so use it to clone the package. Otherwise,

+     # prepend the Fedora dist-git HTTPS URL to it.

+     if grep -q // <<< "$name"; then

+         url="$name"

+         name="${name##*/}"

+     else

+         url="https://src.fedoraproject.org/rpms/$name"

+     fi

+ 

+     echo "Using $url to clone $name and checking out $branch"

+ 

+     # In the following, we assume we're in a clean directory. Since we're just

+     # building an SRPM, we don't need any of the package's dependencies.

+     (

+         set -euo pipefail

+ 

+         # While not necessary, explicitly append .git to the URL we clone.

+         _e git clone "$url.git" "$name"

+ 

+         # Since we're in a subshell, we can cd and not worry about reverting

+         # pwd.

+         cd "$name"

+ 

+         # Check out the branch we parsed, if present.

+         if [ ! -z "$branch" ]; then

+             _e git checkout "$branch"

+         fi

+ 

+         # Build an SRPM and upload it to COPR to build.

+         _e fedpkg srpm

+         _e copr build "$copr" *.src.rpm

+     )

+ }

+ 

+ function review_pr() {

+     # So we're stuck with an ugly hack here.

+ 

+     # Fedora COPR won't let us rebuild packages from the existing dist-git

+     # spec (when passed as a URL), so we're forced to clone the dist-git and

+     # upload a SRPM of the package to COPR to build. Additionally, Pagure lets

+     # users open PRs from external git trees. It will be up to the caller of

+     # this script to specify the correct URL (sans .git suffix) and

+ 

+     # We end up doing this for all packages we need.

+ 

+     if (( $# < 2 )); then

+         echo "Usage: $0 copr-name package [dependent-package...]"

+         echo ""

+         echo "In the COPR repo specified by copr-name, build package and any"

+         echo "provided dependent-packages at the version specified by"

+         echo "dist-git-branch."

+         echo ""

+         echo "package and dependent-package can take the following forms:"

+         echo " - <name> - use the default dist-git repo and default branch"

+         echo " - <name>::<branch> - use the default dist-git repo and specified branch"

+         echo " - <url> - use the specified url and default branch"

+         echo " - <url>::<branch> - use the specified url and specified branch"

+         echo ""

+         echo "Note: if copr-name doesn't exist, this script will create it."

+         exit 1

+     fi

+ 

+     local copr="$1"

+     shift

+ 

+     # If the COPR doesn't exist, create it.

+     _e check_copr_repo "$copr" || exit $?

+ 

+     local tmpdir="/tmp/stewardship-sig-$RANDOM-$RANDOM-$RANDOM"

+     _e mkdir -p "$tmpdir" && cd "$tmpdir"

+ 

+     # We treat all packages equally and build them in the order they were

+     # given to us. In the future, we could build the first package and

+     # if it succeeds, build the rest in parallel. Additionally, we could

+     # check (for any dependent package) whether build failure is truly a

+     # FTBFS without this package (by creating a clean COPR and retrying).

+     # Both of those are more work than this script needs to do.

+     while (( $# > 0 )); do

+         local package="$1"

+         shift

+ 

+         echo "Building $package in $copr"

+         _e build_package "$copr" "$package" || exit 1

+     done

+ 

+     # Only clean up in the event the build succeeded. Otherwise, "litter" so

+     # that we have a local checkout of the package (as that'll likely be the

+     # next step in debugging if the error came from the PR under review and

+     # not from a FTBFS).

+     rm -rf "$tmpdir"

+ }

+ 

+ review_pr "$@"

This script will take a series of packages and build them in a COPR
(creating the repo if it doesn't exist). This acts as a basic sanity
check for updating a package: if it and the specified dependencies build
in a Rawhide COPR, it is likely safe to push this out to all of Rawhide.

Note that this generates a SRPM locally; in that sense, it should be
relatively safe to run, but the remote PR must be inspected prior to
execution of this script.

This PR was used to review the bcel PR.

Signed-off-by: Alexander Scheel <ascheel@redhat.com>

Looks useful - just one question: Why do you pass --enable-net when creating the temporary COPR project? It should never be necessary.

Perhaps a bad habit. I've always done so myself but I can remove it and if
it's necessary for a build, we can always enable it manually.

(I'm also not sure how Requires: mvn(...) works and whether or not that
ends up going to Maven Central to fetch dependencies which don't appear to
be packaged.)

(I'm also not sure how Requires: mvn(...) works and whether or not that
ends up going to Maven Central to fetch dependencies which don't appear to
be packaged.)

No, it doesn't. Fedora packages in Koji never have internet access. When using Copr to check if something would work in Fedora, we should never enable it either.

(I'm also not sure how Requires: mvn(...) works

Requires: mvn(...) and similar (i.e. Requires: pkgconfig(...)) are basically just alternative ways to tell rpm/dnf what packages to install – "give me package that contains necessary files so that maven will consider dependency X as present". These dependencies are considered more accurate and stable (see below) than the actual package names, so they are usually preferred.

The benefit is that if package gets renamed (i.e. X-devel to X-libs-devel or something similar), the special provide mvn(X) will be still provided by the correct package and your spec file will not need an update.

See guidelines for pkgconfig for details; the rationale of mvn(...) should be similar.

rebased onto 22e8e61

2 years ago

Thanks all for the explanations. I've updated and dropped internet connectivity.

Great, thanks. Will merge.

Pull-Request has been merged by decathorpe

2 years ago
Metadata