Copilot commented on code in PR #13535: URL: https://github.com/apache/gluten/pull/13535#discussion_r4484376748
########## .github/workflows/update_fork_build_status.yml: ########## @@ -0,0 +1,182 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# A pull request from a fork runs velox_backend_x86.yml in the fork, on push. This +# workflow reports that run on the pull request as the check named below: it creates +# the check when it first finds the run, and updates it on later passes. It only reads +# run metadata, so it never runs pull request code and needs no pull_request_target. +# Ported from apache/spark's update_build_status.yml (SPARK-60578). +name: Update fork build status + +on: + schedule: + - cron: '*/15 * * * *' + workflow_dispatch: + +permissions: {} + +concurrency: + group: ${{ github.workflow }} + cancel-in-progress: false + +jobs: + update: + if: github.repository == 'apache/gluten' + runs-on: ubuntu-slim Review Comment: `runs-on: ubuntu-slim` is not a standard GitHub-hosted runner label, so this workflow is likely to fail scheduling. Use a supported runner label (e.g., `ubuntu-22.04` or `ubuntu-latest`), or a repo/org runner label that is known to exist. ########## .github/workflows/update_fork_build_status.yml: ########## @@ -0,0 +1,182 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# A pull request from a fork runs velox_backend_x86.yml in the fork, on push. This +# workflow reports that run on the pull request as the check named below: it creates +# the check when it first finds the run, and updates it on later passes. It only reads +# run metadata, so it never runs pull request code and needs no pull_request_target. +# Ported from apache/spark's update_build_status.yml (SPARK-60578). +name: Update fork build status + +on: + schedule: + - cron: '*/15 * * * *' + workflow_dispatch: + +permissions: {} + +concurrency: + group: ${{ github.workflow }} + cancel-in-progress: false + +jobs: + update: + if: github.repository == 'apache/gluten' + runs-on: ubuntu-slim + permissions: + actions: read + checks: write Review Comment: This job uses the GitHub API to list PRs (`pulls.list`) and read existing check-runs. With `permissions: {}` at the workflow level, the job token only has `actions: read` and `checks: write`, which is insufficient to list PRs (needs `pull-requests: read`). Consider adding `pull-requests: read` (and, if required by your org settings, `contents: read`) to avoid 403s at runtime. ########## .github/actions/restore-upstream-ccache/action.yml: ########## @@ -0,0 +1,88 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +name: Restore ccache from apache/gluten main +description: >- + Download the ccache stash that apache/infrastructure-actions/stash/save stored + for `key` on apache/gluten main. stash/restore only searches the repository the + run belongs to, so a fork run uses this after missing its own stash. Without a + stash for `key`, the newest stash for the same key prefix is used, as + actions/cache restore-keys would. +inputs: + key: + description: The key the stash was saved with, ending in a hashFiles() hash. + required: true + path: + description: The directory to restore into. + required: true +outputs: + hit: + description: "'true' if a stash was downloaded." + value: ${{ steps.download.outcome == 'success' }} +runs: + using: composite + steps: + - name: Find the stash + id: find + uses: actions/github-script@v9 + env: + KEY: ${{ inputs.key }} + with: + script: | + const repo = { owner: 'apache', repo: 'gluten' } + // stash/save appends the branch name to the key. Only stashes saved by + // apache/gluten's own runs on main count. + const usable = (a) => !a.expired && a.name.endsWith('-main') + && a.workflow_run.head_branch == 'main' + && a.workflow_run.head_repository_id == a.workflow_run.repository_id + const exact = process.env.KEY + '-main' + const { data } = await github.rest.actions.listArtifactsForRepo({ + ...repo, name: exact, per_page: 10 + }) + let stash = data.artifacts.find(usable) Review Comment: Limiting `listArtifactsForRepo` to `per_page: 10` without pagination can miss the desired stash if there are more than 10 artifacts with the same name (or if older-but-still-valid artifacts exist beyond the first page). To make restores reliable over time, paginate (or raise to `per_page: 100` and paginate if needed) before selecting the first usable artifact. ########## dev/ci-velox-buildstatic-centos-8.sh: ########## @@ -24,6 +24,10 @@ if [ "$(uname -m)" = "aarch64" ]; then export VCPKG_FORCE_SYSTEM_BINARIES=1; fi +# Rewrite absolute paths under the checkout to relative ones, so a ccache saved by +# a checkout at another path still hits. +export CCACHE_BASEDIR=$(cd "$(dirname "$BASH_SOURCE")/.."; pwd) Review Comment: `CCACHE_BASEDIR` should be quoted to avoid word-splitting if the checkout path contains spaces (and to be consistent with other path exports). Consider using a quoted command substitution (and optionally `pwd -P` to canonicalize symlinks). This same pattern appears in the other CI velox build scripts touched by this PR. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
