David Caro has uploaded a new change for review. Change subject: Adapted to new stable branch and improvements ......................................................................
Adapted to new stable branch and improvements - Showing contact address when login error occurs Bug-Url hook: - Skiping bugs out of the ovirt-related products - If the patch is in the stable branch: * -1 if no bug-url * -1 if any target release incorrect * -1 if any private bug set_POST hook: - Warning message when not oVirt product - Warning message when incorrect TR for the branch set_MODIFIED hook: - Warning message when not oVirt product - Warning message when incorrect TR for the branch Change-Id: I233f50c77fc2d419a5bc29b2ee5b3299c4bd6293 Signed-off-by: David Caro <[email protected]> --- M hooks/bz/change-merged.bz.set_MODIFIED M hooks/bz/patchset-created.bz.1.bug_url M hooks/bz/patchset-created.bz.2.add_tracker M hooks/bz/patchset-created.bz.2.set_POST M hooks/lib/bz.sh M hooks/lib/conf.sh M hooks/lib/gerrit.sh 7 files changed, 335 insertions(+), 64 deletions(-) git pull ssh://gerrit.ovirt.org:29418/gerrit-admin refs/changes/00/19600/1 diff --git a/hooks/bz/change-merged.bz.set_MODIFIED b/hooks/bz/change-merged.bz.set_MODIFIED index 731d8c7..fe3ca3b 100755 --- a/hooks/bz/change-merged.bz.set_MODIFIED +++ b/hooks/bz/change-merged.bz.set_MODIFIED @@ -8,11 +8,27 @@ # TRACKER_NAME -> Tracker name as seen on the gerrit external tracker list # BZ_USER -> Username to use when logging in to bugzilla # BZ_PASS -> Password to use when logging into bugzilla +# CHECK_TARGET_RELEASE -> Pairs of 'branch|regexp' to match agains the bug +# target release, if regexp starts with '!' it will be negated # source bz.sh source tools.sh source gerrit.sh source conf.sh + + +finish() +{ + local result="${1:-0}" + if [[ "$result" == "-1" ]]; then + message="$(conf.t_get message)" + gerrit.review "-1" "$message" + echo "$message" + bz.clean + exit 2 + fi +} + ############################################################################### ## MAIN @@ -29,11 +45,21 @@ bug_ids=($(bz.get_bug_id "$commit")) echo "Processing bugs ${bug_ids[@]}" for bug_id in "${bug_ids[@]}"; do - bz.login -b "$bug_id" "$bz_user" "$bz_password" + hdr="::bug $bug_id::" + msg_hdr="\n* Set MODIFIED #$bug_id:" + bz.login -b "$bug_id" "$bz_user" "$bz_password" \ + || { + message+="$msg_hdr Error logging to bugzilla, please contact " + message+="${CONTACT:[email protected]}" + conf.t_put message "$message" + finish "-1" + } ### Skip downstream bugs product="$(bz.get_product "$bug_id")" if ! [[ "$product" =~ ^oVirt$ ]]; then - echo "::bug $bug_id::Not an oVirt bug, belongs to $product" + echo "${hdr}Not an oVirt bug, belongs to $product" + message+="$msg_hdr SKIP, not oVirt prod but $prod" + conf.t_put message "$message" continue fi ## Check if all the external bugs are closed @@ -41,23 +67,46 @@ for patch_id in $(bz.get_external_bugs "$bug_id" "$TRACKER_NAME"); do [[ "$current_patch_id" == "$patch_id" ]] && continue if $(gerrit.is_open "$patch_id"); then - echo "::bug $bug_id::Related patch $patch_id is still open" + echo "${hdr}Related patch $patch_id is still open" all_merged="false" + message+="$msg_hdr $patch_id is still open" fi done if [[ "$all_merged" != "true" ]]; then - echo "::bug $bug_id::Skipping because not all related patches are closed." + echo "${hdr}SKIP because not all related patches are" \ + "closed." + message+="$msg_hdr SKIP, some externals are still open" + conf.t_put message "$message" continue fi + ## Check if the target_release should be checked + echo "${hdr}Checking target release" + res="$(bz.check_target_release \ + "$bug_id" \ + "$branch" \ + "${CHECK_TARGET_RELEASE[@]}")" + if [[ $? -ne 0 ]]; then + echo "${hdr}Target release check failed, skipping bug" + message+="$msg_hdr SKIP, target rel $res" + conf.t_put message "$message" + continue + fi + echo "::bug $bug_id::Target release OK" ## Modify the bug status if res=$(bz.update_status "$bug_id" "MODIFIED" "$commit"); then echo "::bug $bug_id::Status updated on bug #$bug_id for gerrit id" \ "#${change_url//*\/} to MODIFIED${res:+\n $res}" + message+="$msg_hdr OK${res:+, $res}" else - echo "::bug $bug_id::Failed to update the status of bug #$bug_id for"\ + echo "::bug $bug_id::Failed to update the status of bug #$bug_id for" \ "gerrit id #${current_patch_id} to MODIFIED\n $res" + message+="$msg_hdr FAILED, $res" fi + conf.t_put message "$message" done +message="$(conf.t_get message)" +gerrit.review "0" "$message" +echo "Got good review" bz.clean gerrit.clean exit 0 diff --git a/hooks/bz/patchset-created.bz.1.bug_url b/hooks/bz/patchset-created.bz.1.bug_url index 708a319..7240684 100755 --- a/hooks/bz/patchset-created.bz.1.bug_url +++ b/hooks/bz/patchset-created.bz.1.bug_url @@ -8,6 +8,12 @@ # be tested as a bash array: BRANCHES=('rhev-3.2' 'rhev-3.1') # BZ_USER - Bz username to use when logging in # BZ_PASS - Bz password to use when logging in +# PRODUCTS - List of the products allowed +# STABLE_BRANCH - Name of the stable branch to do the extra checks +# CHECK_TARGET_RELEASE -> Pairs of 'branch|regexp' to match agains the bug +# target release, if regexp starts with '!' it will be negated +# CONTACT - email/s for the contact, if not specified, will use +# [email protected] ############################################################################### source bz.sh @@ -23,35 +29,78 @@ ## Parse the configuration conf.load +finish() +{ + local result="${1:-0}" + if [[ "$result" == "-1" ]]; then + message="$(conf.t_get message)" + gerrit.review "-1" "$message" + echo "$message" + bz.clean + exit 2 + fi +} + message="" +result="0" ## Check if we have to manage that branch if ! vindex=$(tools.is_in "$branch" "${BRANCHES[@]}"); then echo "Ignoring branch $branch" exit 2 fi bug_ids=($(bz.get_bug_id $commit)) -## break the chain if no bug-url found if [[ -z "$bug_ids" ]]; then - exit 2 + ## If we are in the stable branch, fail if there are no bug-urls + if [[ "$branch" == "$STABLE_BRANCH" ]]; then + message+="\n* Bug-Url: ERROR, At least one bug-url is required for" + message+=" the stable branch" + conf.t_put message "$message" + finish "-1" + else + ## if we are not, just ignore + exit 2 + fi fi bz_password="${BZ_PASS?No BZ_PASS in the config}" bz_user="${BZ_USER?No BZ_USER in the config}" -bz.login "$bz_user" "$bz_password" -failed="false" +bz.login "$bz_user" "$bz_password" \ +|| { + message+="\nThere was an error logging into bugzilla, please contact " + message+="the administrator ${CONTACT:[email protected]}" + conf.t_put message "$message" + finish "-1" +} echo "Got bug ids ${bug_ids[@]}" for bug_id in ${bug_ids[@]}; do - if bz.is_private "$bug_id"; then - echo "$bug_id::ERROR bug is private." - message+="* Bug-Url::$bug_id::ERROR, private bug" - failed="true" + prod="$(bz.get_product "$bug_id")" + ## If it has the wrong product, ignore + if ! tools.is_in "$prod" "${PRODUCTS[@]}"; then + echo "$bug_id::ERROR Wrong product $prod" + message+="\n* Bug-Url #$bug_id: SKIP, external product" continue fi + ## If it's private, fail + if [[ -z "$SKIP_PRIVATE" ]] && bz.is_private "$bug_id"; then + echo "$bug_id::ERROR bug is private." + message+="\n* Bug-Url #$bug_id: ERROR, private bug" + failed="-1" + continue + fi + ## If we are in the stable branch and the target release is not correct, + ## fail + if [[ "$branch" == "$STABLE_BRANCH" ]]; then + res="$(bz.check_target_release \ + "$bug_id" \ + "$branch" \ + "${CHECK_TARGET_RELEASE[@]}")" + if [[ $? -ne 0 ]]; then + failed="-1" + message+="\n* Bug-Url #$bug_id: ERROR for stable branch target " + message+="rel $res" + continue + fi + fi done -if [[ "$failed" == "true" ]]; then - bz.clean - gerrit.review "-1" "$message" - echo "$message" - exit 2 -else - conf.t_put message "$message" -fi +conf.t_put message "$message" + +finish "$failed" diff --git a/hooks/bz/patchset-created.bz.2.add_tracker b/hooks/bz/patchset-created.bz.2.add_tracker index a8dd679..3b7fe7c 100755 --- a/hooks/bz/patchset-created.bz.2.add_tracker +++ b/hooks/bz/patchset-created.bz.2.add_tracker @@ -8,25 +8,56 @@ # bugzilla submit form. # 81 -> oVirt gerrit # 82 -> RHEV gerrit +# US_PRODUCT - Name of the downstream product ############################################################################### source bz.sh source gerrit.sh source conf.sh +finish() +{ + local result="${1:-0}" + if [[ "$result" == "-1" ]]; then + message="$(conf.t_get message)" + gerrit.review "-1" "$message" + echo "$message" + bz.clean + exit 2 + fi +} + + add_tracker() { local bug_id="${1?}" - # Get the bug url and it's flags - bz.login -b "$bug_id" "$bz_user" "$bz_password" - ## Update the tracker on the bz bug local message="$(conf.t_get message)" + local hdr="::bug $bug_id::" + # Get the bug url and it's flags + bz.login -b "$bug_id" "$bz_user" "$bz_password" \ + || { + message+="$msg_hdr Error logging to bugzilla, please contact " + message+="${CONTACT:[email protected]}" + conf.t_put message "$message" + finish "-1" + } + ## Skip bugs with wrong product + prod="$(bz.get_product "$bug_id")" + if ! [[ "$prod" =~ ^$US_PRODUCT$ ]]; then + echo "${hdr}Ignoring, wrong product $prod" + message+="\n* Tracker #$bug_id: SKIP, not $US_PRODUCT prod" + conf.t_put message "$message" + return 0 + fi + ## Update the tracker on the bz bug if ! bz.add_tracker "$bug_id" "${TRACKER_ID?}" "${change_url//*\/}"; then - ## raise a warning error - message+="\n* Tracker #$bug_id: FAILED" - echo "WARNING: Failed to update tracker on bug #$bug_id for gerrit id #${change_url//*\/}" + message+="\n* Tracker #$bug_id: FAILED, please contact " + message+="${CONTACT:[email protected]}" + echo "WARNING: Failed to update tracker on bug #$bug_id for gerrit" \ + "id #${change_url//*\/}" else message+="\n* Tracker #$bug_id: OK" - echo "Tracker updated on bug #$bug_id for gerrit id #${change_url//*\/}" + echo "Tracker updated on bug #$bug_id for gerrit id" \ + "#${change_url//*\/}" fi conf.t_put message "$message" return 0 diff --git a/hooks/bz/patchset-created.bz.2.set_POST b/hooks/bz/patchset-created.bz.2.set_POST index 8815d93..c0b9c7d 100755 --- a/hooks/bz/patchset-created.bz.2.set_POST +++ b/hooks/bz/patchset-created.bz.2.set_POST @@ -1,31 +1,82 @@ #!/bin/bash ############################################################################### # Moves the bugs in the commit message to POST state +# +# Configuration variable +# CHECK_TARGET_RELEASE -> Pairs of 'branch|regexp' to match agains the bug +# target release, if regexp starts with '!' it will be negated +# US_PRODUCT - Name of the downstream product ############################################################################### source bz.sh source gerrit.sh source conf.sh + +finish() +{ + local result="${1:-0}" + if [[ "$result" == "-1" ]]; then + message="$(conf.t_get message)" + gerrit.review "-1" "$message" + echo "$message" + bz.clean + exit 2 + fi +} + + set_POST() { local bug_id="${1?}" local commit="${2?}" - ## Get the bug url and it's flags - bz.login -b "$bug_id" "$bz_user" "$bz_password" - ## Update the tracker on the bz bug local message="$(conf.t_get message)" + local hdr="::bug $bug_id::" + local msg_hdr="\n* Set POST #$bug_id:" + ## Get the bug url and it's flags + bz.login -b "$bug_id" "$bz_user" "$bz_password" \ + || { + message+="$msg_hdr Error logging to bugzilla, please contact " + message+="${CONTACT:[email protected]}" + conf.t_put message "$message" + finish "-1" + } + ## Skip bugs with wrong product + prod="$(bz.get_product "$bug_id")" + if ! [[ "$prod" =~ ^$US_PRODUCT$ ]]; then + echo "${hdr}Ignoring, wrong product $prod" + message+="$msg_hdr SKIP, not $US_PRODUCT prod" + conf.t_put message "$message" + return 0 + fi + ## check if the target_release is correct + echo "${hdr}Checking target release" + res="$(bz.check_target_release \ + "$bug_id" \ + "$branch" \ + "${CHECK_TARGET_RELEASE[@]}")" + if [[ $? -ne 0 ]]; then + echo "${hdr}Target release check failed, skipping bug" + message+="$msg_hdr SKIP, target rel $res" + conf.t_put message "$message" + return 0 + fi + echo "${hdr}Target release OK" + ## Update the tracker on the bz bug if ! res=$(bz.update_status "$bug_id" "POST" "$commit"); then - message+="\n* Set POST #$bug_id: FAILED, $res" - echo "Failed to update the status of bug #$bug_id for gerrit id #${change_url//*\/} to POST" + message+="$msg_hdr FAILED, $res" + echo "Failed to update the status of bug #$bug_id for gerrit id" \ + "#${change_url//*\/} to POST" echo " $res" else - message+="\n* Set POST #$bug_id: OK${res:+, $res}" - echo "Status updated on bug #$bug_id for gerrit id #${change_url//*\/} to POST" + message+="$msg_hdr OK${res:+, $res}" + echo "Status updated on bug #$bug_id for gerrit id" \ + "#${change_url//*\/} to POST" fi conf.t_put message "$message" return 0 } + ############################################################################### ## MAIN ## Parse the parameters diff --git a/hooks/lib/bz.sh b/hooks/lib/bz.sh index 38f42f1..6c463b3 100644 --- a/hooks/lib/bz.sh +++ b/hooks/lib/bz.sh @@ -181,16 +181,20 @@ server_url="${server_url}${bug_id:+/show_bug.cgi?id=${bug_id}}" shift $((OPTIND - 1)) local cookie_jar="$(conf.t_get bz_cookie_jar)" - if ! [[ -f "$cookie_jar" ]] ; then + if ! [[ -f "$cookie_jar" ]] \ + || ! grep -q 'Bugzilla_logincookie' "$cookie_jar"; then local bz_user="${1?No user passed}" local bz_password="${2?No password passed}" [[ "$cookie_jar" == "" ]] \ && cookie_jar="/tmp/bz_cache.$PPID.cookies" conf.t_put "bz_cookie_jar" "$cookie_jar" - wget -qO ${plain_bug:-/dev/null} --save-cookies "$cookie_jar" \ + res="$(wget -qO ${plain_bug:--} --save-cookies "$cookie_jar" \ --post-data "Bugzilla_login=${bz_user//@/%40}&Bugzilla_password=${bz_password}&GoAheadAndLogIn=Log+in" \ - "$server_url" + "$server_url")" fi + ## Return false if we did not login + grep -q 'Bugzilla_logincookie' "$cookie_jar" \ + || { echo -e "::bz.login::Failed to log in\n$res" && return 1; } } @@ -281,7 +285,7 @@ # bz.get_token() { - bug_id=${1?No bug id passed} + local bug_id=${1?No bug id passed} bz.get_bug -p "$bug_id" \ | grep -Po "(?<=<input type=\"hidden\" name=\"token\" value=\")[^\"]*" } @@ -301,9 +305,9 @@ # Add a new external bug to the external bugs list bz.add_tracker() { - bug_id="${1?}" - tracker_id="${2?}" - external_id="${3?}" + local bug_id="${1?}" + local tracker_id="${2?}" + local external_id="${3?}" bz.update_bug "$bug_id" \ "external_bug_id=${external_id}" \ "external_id=${tracker_id}" @@ -312,8 +316,8 @@ ## Update fixed in version field bz.update_fixed_in_version() { - bug_id="${1?}" - fixed_in_version="${2?}" + local bug_id="${1?}" + local fixed_in_version="${2?}" bz.update_bug "$bug_id" "cf_fixed_in=${fixed_in_version}" } @@ -321,9 +325,9 @@ ## Update fixed in version field bz.update_status_and_version() { - bug_id="${1?}" - bug_status="${2?}" - fixed_in_version="${3?}" + local bug_id="${1?}" + local bug_status="${2?}" + local fixed_in_version="${3?}" bz.update_bug "$bug_id" \ "bug_status=${bug_status}" \ "cf_fixed_in=${fixed_in_version}" @@ -342,37 +346,37 @@ # # # Legal status transitions: -# ASSIGNED -> POST +# NEW|ASSIGNED|MODIFIED -> POST # POST -> MODIFIED # # If it's a revert any source status is allowed # bz.update_status() { - bug_id="${1?}" - new_status="${2?}" - commit_id="${3?}" - current_status="$(bz.get_bug_status "$bug_id")" + local bug_id="${1?}" + local new_status="${2?}" + local commit_id="${3?}" + local current_status="$(bz.get_bug_status "$bug_id")" if [[ "$current_status" == "$new_status" ]]; then echo "already on $new_status" return 0 fi if ! bz.is_revert "$commit_id"; then case $current_status in - ASSIGNED) + ASSIGNED|NEW|MODIFIED) if [[ "$new_status" != "POST" ]]; then - echo "ilegal change from $current_status" + echo "illegal change from $current_status" return 1 fi ;; POST) if [[ "$new_status" != "MODIFIED" ]]; then - echo "ilegal change from $current_status" + echo "illegal change from $current_status" return 1 fi ;; *) - echo "ilegal change from $current_status" + echo "illegal change from $current_status" return 1 esac fi @@ -396,8 +400,8 @@ # bz.get_external_bugs() { - bug_id="${1?}" - external_name="${2:-[^.*]}" + local bug_id="${1?}" + local external_name="${2:-[^.*]}" bz.get_bug "$bug_id" \ | grep -Po "(?<=<external_bugs name=\"$external_name\">)\d*" } @@ -423,21 +427,106 @@ # Return the product name of the given bug bz.get_product() { - bug_id="${1?}" + local bug_id="${1?}" bz.get_bug "$bug_id" \ | grep -Po "(?<=<product>)[^<]*" } - ###### # Usage: # bz.is_private_id # -# return 0 if it's private, 1 otherwise +# Return 0 if it's private, 1 otherwise bz.is_private() { - bug_id="${1?}" + local bug_id="${1?}" bz.get_bug "$bug_id" \ | grep -q '<group id="218">private</group>' } + + +###### +# Usage: +# bz.get_target_release bug_id +# +# Return the target release of the bug +bz.get_target_release() +{ + local bug_id="${1?}" + bz.get_bug "$bug_id" \ + | grep -Po "(?<=<target_release>)[^<]*" +} + + +###### +# Usage: +# bz.check_target_release bug_id branch tr_match [tr_match [...]] +# +# bug_id +# Id of the bug to get the target_release from +# +# branch +# Name of the current branch +# +# tr_match +# Tuple in the form 'branch_name|[!]regexp' +# +# branch_name +# name of th ebranch that should check the regexp +# +# [!]regexp +# regular expresion to match the target release agains, if preceded +# with '!' the expression will be negated +# +# Example: +# +# bz.check_target_release 1234 master 'master|3\.3.*' 'master|!3\.[21].*' +# +# That will check that the bug 1234 target release matches: +# 3\.3.* +# And does not match: +# 3\.3\.0\..* +# +# So 3.3.0 or 3.3 will pass but 3.2 and 3.3.0.1 will not +# +# +# Return false if the target release and branch defined in tr_match +# configuration variable do not match the given bug's +# target release +bz.check_target_release() +{ + local bug_id="${1?}" + local branch="${2?}" + local br_reg_pairs=("${@:3}") + local hdr="::bug $bug_id::bz.check_target_release::" + ## Check if the target_release should be checked + for br_reg in "${br_reg_pairs[@]}"; do + if ! [[ $br_reg =~ ^${branch}\|.* ]]; then + #not for this branch + continue + fi + echo "${hdr}TR has to match $br_reg" >&2 + regexp="${br_reg#*|}" + target_release="$(bz.get_target_release "$bug_id")" + if [[ "${regexp:0:1}" == '!' ]]; then + ## Negate the regexp + regexp="${regexp:1}" + if [[ "$target_release" =~ ^$regexp$ ]]; then + echo "${hdr}target release should not match match" \ + "$regexp but it is $target_release" >&2 + echo "$target_release should not match $regexp" + return 1 + fi + else + if ! [[ "$target_release" =~ $regexp ]]; then + echo "${hdr}target release should match $regexp but" \ + "it is $target_release" >&2 + echo "$target_release should match $regexp" + return 1 + fi + fi + echo "${hdr}Bug tr matches $regexp, it's $target_release" >&2 + done + return 0 +} diff --git a/hooks/lib/conf.sh b/hooks/lib/conf.sh index dfb3214..c90c37c 100644 --- a/hooks/lib/conf.sh +++ b/hooks/lib/conf.sh @@ -134,7 +134,7 @@ if [[ -f "$conf_file" ]] && grep -Eq "^\s*$name=" "$conf_file"; then ## delete the old entry local entry="$(grep "$name=" "$conf_file")" - if [[ "${entry: -1}" == '"' ]]; then + if [[ "$entry" =~ $name=\".*\"$ ]]; then sed -i -e "/$name=/d" "$conf_file" else ## It's multiline diff --git a/hooks/lib/gerrit.sh b/hooks/lib/gerrit.sh index 8791a99..4282824 100644 --- a/hooks/lib/gerrit.sh +++ b/hooks/lib/gerrit.sh @@ -60,11 +60,13 @@ gerrit.review() { local result=${1?} - local message="$( echo "${2?}" | fold -w 60 -s )" +# local message="$( echo "${2?}" | fold -w 80 -s )" + local message="${2?}" local project=${3:-$project} local commit=${4:-$commit} local footer=" help:$HELP_URL" + echo "Message:\n$message" ssh "${GERRIT_SRV?}" -p 29418 gerrit review \ --verified="$result" \ --message="\"$message${HELP_URL:+$footer}\"" \ -- To view, visit http://gerrit.ovirt.org/19600 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I233f50c77fc2d419a5bc29b2ee5b3299c4bd6293 Gerrit-PatchSet: 1 Gerrit-Project: gerrit-admin Gerrit-Branch: master Gerrit-Owner: David Caro <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
