Skip to content

Conversation

@kwankyu
Copy link
Collaborator

@kwankyu kwankyu commented Dec 2, 2025

Fixes #40929

Test: https://github.com/kwankyu/sage/actions/runs/19849361332/job/56872883527

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

⌛ Dependencies

@kwankyu kwankyu changed the title Fix no-space-left error in doc-build Fix no-space-left error in doc-build workflow Dec 2, 2025
@kwankyu kwankyu changed the title Fix no-space-left error in doc-build workflow Fix no-space-left failures in doc-build workflow Dec 2, 2025
@kwankyu kwankyu marked this pull request as ready for review December 2, 2025 07:32
@github-actions
Copy link

github-actions bot commented Dec 2, 2025

Documentation preview for this PR (built with commit 3a859e5; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@cxzhong
Copy link
Contributor

cxzhong commented Dec 2, 2025

It seems OK now. why block this?

@kwankyu
Copy link
Collaborator Author

kwankyu commented Dec 2, 2025

It seems OK now.

What is "it"?

why block this?

Are you asking why this PR is a blocker?

The doc-build workflow is failing: https://github.com/sagemath/sage/actions/workflows/doc-build.yml

This PR fixes it, and needs to be merged quickly. Hence a blocker.

@cxzhong
Copy link
Contributor

cxzhong commented Dec 2, 2025

https://doc-pr-41248--sagemath.netlify.app/html/en/
This document preview is OK. I approve it can be merged. But I left @tobiasdiez to decide it. I do not know much about Workflows.

@cxzhong cxzhong requested a review from tobiasdiez December 2, 2025 16:13
@vincentmacri vincentmacri added p: CI fix merged before running CI tests github actions Pull requests that update GitHub Actions code and removed p: blocker / 1 labels Dec 2, 2025
@vincentmacri
Copy link
Member

Blocker label isn't necessary since we have the CI fix label now.

@vincentmacri
Copy link
Member

Thank you!

@kwankyu
Copy link
Collaborator Author

kwankyu commented Dec 2, 2025

Thanks!

Blocker label isn't necessary since we have the CI fix label now.

Let me clarify.

"p: CI Fix" priority label is for PRs that needs to be merged to the PR branch before CI workflows run (since otherwise CI workflows would fail).

"p: blocker" priority label is for PRs that should be merged to the next release since it is very critical for further development. Hence the release manager (@vbraun) prioritize these PRs when he starts to select positively-reviewed PRs to merge to the next release.

If I remember correctly:

  • Originally "p: blocker" was for PRs that should be merged before the final stable release is made.
  • For some time, the functions of "p: CI Fix" and "p: blocker" were combined (in "p: blocker") but later separated as they are now.

The caveat is that a PR making changes to github workflows is not effective even if it has "p: CI Fix" label since github action machinery uses workflows from the PR branch (this PR is, of course, different from the PR making changes to github workflows).

Hence "p: blocker" is the label this PR should get.

These things should be explained in https://doc-release--sagemath.netlify.app/html/en/developer/review

@vincentmacri
Copy link
Member

The caveat is that a PR making changes to github workflows is not effective even if it has "p: CI Fix" label since github action machinery uses workflows from the PR branch (this PR is, of course, different from the PR making changes to github workflows).

I'm not quite sure I understand the point of "p: CI Fix" then, but okay. Does it only help for CI fixes that do not change code under .github/?

These things should be explained in https://doc-release--sagemath.netlify.app/html/en/developer/review

Yes. There are many things that ought to be better explained in our developer guide.

@kwankyu
Copy link
Collaborator Author

kwankyu commented Dec 2, 2025

The caveat is that a PR making changes to github workflows is not effective even if it has "p: CI Fix" label since github action machinery uses workflows from the PR branch (this PR is, of course, different from the PR making changes to github workflows).

I'm not quite sure I understand the point of "p: CI Fix" then, but okay.

Well, the label is not to fix the workflows, but to fix other things so that CI workflows work.

Does it only help for CI fixes that do not change code under .github/?

Close. "p: CI Fix" is effective for all CI fixes that make changes to files other than .yml files in .github/workflows/.

@vincentmacri
Copy link
Member

Close. "p: CI Fix" is effective for all CI fixes that make changes to files other than .yml files in .github/workflows/.

Good to know! I thought "p: CI Fix" was effective for all changes. Sorry about that!

@kwankyu
Copy link
Collaborator Author

kwankyu commented Dec 2, 2025

Indeed, that is a very subtle point.

By the way, I think the label should be renamed to "p: CI fix" since all other labels are in small letters.

@kwankyu
Copy link
Collaborator Author

kwankyu commented Dec 3, 2025

The other doc build workflows (doc-build-pdf.yml and doc-build-livedoc) suffer from the same issue:

I applied the same fix to these workflows too.

vbraun pushed a commit to vbraun/sage that referenced this pull request Dec 4, 2025
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Fixes sagemath#40929

Test:
https://github.com/kwankyu/sage/actions/runs/19849361332/job/56872883527

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->

URL: sagemath#41248
Reported by: Kwankyu Lee
Reviewer(s): Chenxin Zhong, Kwankyu Lee, Vincent Macri
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c: scripts github actions Pull requests that update GitHub Actions code p: blocker / 1 p: CI fix merged before running CI tests s: positive review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Doc builds failing due to space issue

5 participants