From 49eeb1e8a0399db3ef316603a08776100957c217 Mon Sep 17 00:00:00 2001 From: Adrian Pop Date: Mon, 24 Aug 2026 18:13:48 +0200 Subject: [PATCH] Say what is wrong with a pull request number, and test the head ref (#307) Asking the job for 16360 spent minutes cloning and resetting the OpenModelica repository before failing with "Could not fetch refs/pull/16360/merge: either there is no such pull request, or GitHub cannot merge it into its base branch". Both of those were wrong about what had happened: 16360 is an *issue*. Issues and pull requests share one numbering on GitHub, so an issue number reaches the job looking exactly like a pull request number, and the answer to that is "there is no such pull request", which the message buried as one of two possibilities. The stage now asks GitHub what the number is - one git ls-remote, before the clone, the reset and the build - and says so: OpenModelica/OpenModelica has no pull request 16360. Issues and pull requests share one numbering there, so check that 16360 is not the number of an issue. The checkout separates the two cases as well. GitHub only has refs/pull//merge while it can merge the pull request into its base branch, so a pull request that conflicts, or one already closed, has only refs/pull//head. That is still something to test - the pull request on its own rather than as it would land - so it is used, with a warning saying which of the two was tested and what the head ref does not have. Nothing at all for that number is still an error. While here: a parameter is only recorded on a job once a build has run with its definition, so the build that first sees this file has the new ones as null, and every stage that reads them to decide whether to run would fail on it, not only a pull request one. They now fall back to what the definition says the default is. --- Generated by Claude Code. --- .CI/Jenkinsfile | 60 +++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 48 insertions(+), 12 deletions(-) diff --git a/.CI/Jenkinsfile b/.CI/Jenkinsfile index fec892a..c85047a 100644 --- a/.CI/Jenkinsfile +++ b/.CI/Jenkinsfile @@ -124,25 +124,34 @@ pipeline { stage('pull request') { agent { node { - label "${params.pull_request_node}" + label "${params.pull_request_node ?: 'ryzen-5950x-1'}" customWorkspace 'ws/OpenModelicaLibraryTestingWork' } } options { skipDefaultCheckout() } when { beforeAgent true - expression { params.pull_request.trim() } + expression { pullRequest() } } steps { script { - if (!(params.pull_request.trim() ==~ /[0-9]+/)) { + if (!(pullRequest() ==~ /[0-9]+/)) { error "pull_request is a pull request number; got '${params.pull_request}'" } } + // Before the clone, the reset and the build: a number that is not a + // pull request costs minutes to find out about otherwise. Issues and + // pull requests share one numbering, so an issue number gets this far. + sh """ + if ! git ls-remote --exit-code https://github.com/OpenModelica/OpenModelica.git 'refs/pull/${pullRequest()}/*' > /dev/null; then + echo "OpenModelica/OpenModelica has no pull request ${pullRequest()}. Issues and pull requests share one numbering there, so check that ${pullRequest()} is not the number of an issue." + exit 1 + fi + """ // One build of omc per pull request is kept, as for a branch, and they // accumulate: a pull request is tested once and never again. sh 'find "$HOME/saved_omc" -maxdepth 1 -name "pr-*" -type d -mtime +14 -exec rm -rf {} ";" || true' - runRegressiontest("pr-${params.pull_request.trim()}", "pr-${params.pull_request.trim()}", '', '', false, '', '', false, false, 0, params.pull_request_config) + runRegressiontest("pr-${pullRequest()}", "pr-${pullRequest()}", '', '', false, '', '', false, false, 0, params.pull_request_config ?: 'configs/conf.json') } } @@ -588,7 +597,7 @@ pipeline { } when { beforeAgent true - expression { params.drop_stale_pull_request_tables } + expression { params.drop_stale_pull_request_tables ?: false } } environment { PGPASSFILE = credentials('omdb-pgpass') @@ -612,7 +621,7 @@ pipeline { } when { beforeAgent true - expression { params.pull_request.trim() } + expression { pullRequest() } } environment { PYTHONIOENCODING = 'utf-8' @@ -620,12 +629,12 @@ pipeline { } steps { script { - if (!(params.pull_request.trim() ==~ /[0-9]+/)) { + if (!(pullRequest() ==~ /[0-9]+/)) { error "pull_request is a pull request number; got '${params.pull_request}'" } } sh 'rm -rf history' - sh "./pr-report.py '${params.pull_request.trim()}' --baseline='${params.pull_request_baseline.trim()}'" + sh "./pr-report.py '${pullRequest()}' --baseline='${(params.pull_request_baseline ?: 'master').trim()}'" // The summary to comment on the pull request with, in the build log // until there is a token to post it with. sh 'cat history/pr-*/00_comment.md' @@ -634,6 +643,20 @@ pipeline { } } } +/** + * The pull request this job is testing, or "" when it is testing branches. + * + * A job only learns of a parameter that has been added to it once a build has + * run with the definition, so the first build after this file changes sees the + * new ones as null - which is every build of the pipeline, not only one asking + * for a pull request, because the stages that ignore them still have to decide + * whether to run. Everything that reads them therefore falls back to what the + * definition says the default is. + */ +def pullRequest() { + return (params.pull_request ?: '').trim() +} + def omsimulatorHash() { return 'master' } @@ -878,10 +901,23 @@ def runRegressiontest(branch, name, extraFlags, omsHash, omcompiler, extrasimfla // same workspace to trip over. def pullRequest = branch.startsWith('pr-') && branch.substring(3).isInteger() ? branch.substring(3) : '' def checkoutRef = pullRequest ? """ - if ! git fetch --force https://github.com/OpenModelica/OpenModelica.git refs/pull/${pullRequest}/merge; then - echo "Could not fetch refs/pull/${pullRequest}/merge: either there is no such pull request, or GitHub cannot merge it into its base branch." - exit 1 - fi + REFS=`git ls-remote https://github.com/OpenModelica/OpenModelica.git "refs/pull/${pullRequest}/head" "refs/pull/${pullRequest}/merge"` || exit 1 + case "\$REFS" in + *"refs/pull/${pullRequest}/merge"*) + PRREF="refs/pull/${pullRequest}/merge" ;; + *"refs/pull/${pullRequest}/head"*) + # GitHub only has a merge ref while it can merge the pull request into + # its base branch. Without one there is still something to test, only it + # is the pull request on its own rather than as it would land. + echo "WARNING: pull request ${pullRequest} has no merge ref: it conflicts with its base branch, or it is closed." + echo "WARNING: testing refs/pull/${pullRequest}/head, which does not have what was merged into the base branch since it was branched." + PRREF="refs/pull/${pullRequest}/head" ;; + *) + echo "OpenModelica/OpenModelica has no pull request ${pullRequest}." + exit 1 ;; + esac + echo "Testing \$PRREF" + git fetch --force https://github.com/OpenModelica/OpenModelica.git "\$PRREF" || exit 1 git checkout -f --detach FETCH_HEAD || exit 1 git fetch --tags --force || exit 1 """ : """