Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test on CI that update-meson is properly ran #39527

Merged
merged 7 commits into from
Feb 21, 2025
+30 −10
Merged

Test on CI that update-meson is properly ran #39527

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
92bc994
Reformat meson.build file to satisfy update-meson
user202729 Feb 14, 2025
82a439a
Test on CI that update-meson is properly ran
user202729 Feb 14, 2025
b7ae258
Refactor test-git-no-uncommitted-changes to independent script
user202729 Feb 17, 2025
44e7572
Show newly created files in git diff, if any
user202729 Feb 17, 2025
c370580
Merge two report steps
user202729 Feb 17, 2025
d796a29
Disable running test step when update-meson check fails
user202729 Feb 19, 2025
6bf8c80
Update an outdated comment
user202729 Feb 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions .github/workflows/ci-meson.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,23 @@ jobs:
# Use --no-deps and pip check below to verify that all necessary dependencies are installed via conda
pip install --no-build-isolation --no-deps --config-settings=builddir=builddir . -v

- name: Check update-meson
# this step must be after build, because meson.build creates a number of __init__.py files
# that is needed to make tools/update-meson.py run correctly
shell: bash -l {0}
id: check_update_meson
run: |
python3 tools/update-meson.py
make test-git-no-uncommitted-changes
continue-on-error: true

- name: Show files changed by update-meson
if: ${{ steps.check_update_meson.outcome == 'failure' }}
shell: bash -l {0}
run: |
git status
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be combined with the previous step.

Copy link
Contributor Author

@user202729 user202729 Feb 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find some other good way to ensure simultaneously

  • test step is not run when build step (or some previous step) fails
  • test step is still run when update-meson step fails
  • update-meson is ran reasonably early so that in principle someone want to check the failure of this early can do so (though if it's after build step it is kind of moot anyway)

also the purpose of git diff is to print out the diff, there's no need to print out the diff when there's no change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively we could just remove the whole continue-on-error and let the test not run when update-meson is not ran (which might not be that bad of an idea? If pull request author is quickly notified then they can quickly fix the thing and repush)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively we could just remove the whole continue-on-error and let the test not run when update-meson is not ran (which might not be that bad of an idea? If pull request author is quickly notified then they can quickly fix the thing and repush)

I would prefer this yes. It's the simplest method to maintain and also the one with the least surprising outcomes (if you forget to add a python file in the meson build, then you almost surely get 'confusing' test failures). Alternatively, you could argue that it's a kind of "linter"/checker and thus should be moved to the Lint workflow.

git diff

- name: Verify dependencies
shell: bash -l {0}
run: pip check
Expand All @@ -83,6 +100,14 @@ jobs:
rm -R ./src/sage_setup/
./sage -t --all -p4

- name: Report update-meson failure
if: ${{ steps.check_update_meson.outcome == 'failure' }}
shell: bash -l {0}
run: |
# Please see step 'Show files changed by update-meson' above and apply the changes,
# or run tools/update-meson.py locally
false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite understand the purpose of this check. Why would we want to have two steps that fail due to one reason?


- name: Upload log
uses: actions/[email protected]
if: failure()
Expand Down
5 changes: 5 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -467,3 +467,8 @@ src/sage/libs/mpfr/__init__.py
src/sage/libs/mpc/__init__.py
src/sage/calculus/transforms/__init__.py
src/sage/calculus/__init__.py

# Temporary files generated by Meson CI (needed to make test pass because
# ci-meson.yml runs a `make test-git-no-uncommitted-changes` step)
/.ccache
/setup-miniconda-patched-environment-*.yml
4 changes: 1 addition & 3 deletions src/sage/data_structures/meson.build
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,7 @@ foreach name, pyx : extension_data
)
endforeach

extension_data_cpp = {
'pairing_heap' : files('pairing_heap.pyx'),
}
extension_data_cpp = {'pairing_heap' : files('pairing_heap.pyx')}

foreach name, pyx : extension_data_cpp
py.extension_module(
Expand Down
Loading