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.

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

fix: docs pages #1316

Merged
merged 4 commits into from
Nov 18, 2024
Merged

fix: docs pages #1316

merged 4 commits into from
Nov 18, 2024

Conversation

jjmaestro
Copy link
Contributor

fix: deprecated versions of actions/upload-artifact

The pages workflow is failing due to deprecated versions of actions/upload-artifact.

I believe this is why the docs are not updating (e.g. the Meson docs are still blank after #1060 landed).

For more info, see:

chore: bump version.bzl to the current released version

This is also needed for docs generation, see docs/tools/workspace_status.sh

docs: update releases

@jjmaestro
Copy link
Contributor Author

I was hoping this would work... but there's a bunch of checks failing for the generation of the docs.

I tried to test the workflow in my fork but I wasn't able to :S @jsharpe if you know how to do it, I'll be happy to try.

@jsharpe
Copy link
Member

jsharpe commented Nov 6, 2024

I don't actually know how the doc generation works - @UebelAndre put it together. I believe the issue though is that its trying to run the latest version of stardoc on older branches to generate the docs for older releases and so its failing because bzlmod syntax such as @@ didn't exist when the earlier releases were done.

@jjmaestro
Copy link
Contributor Author

Yeah, I was checking the checks and it does seem like it was doing that, e.g. running Bazel 7.4 with Bzlmod on the 0.7.0 branch and failing... it's a pity cos it stops the other checks from running with more recent tags, e.g. 0.12.0 :-/

For the time being, I'm removing the old releases to see if the checks pass for the new stuff, and if so, I'll revert the change and we can at least know that it works for the new stuff 😅

@jjmaestro
Copy link
Contributor Author

jjmaestro commented Nov 6, 2024

OK, so this worked for the latest releases 🎉 Now, there's some other stuff failing...

Huh! some checksum is failing? 😮

(...)
Error downloading https://download.gnome.org/sources/glib/2.77/glib-2.77.0.tar.xz (...)
Checksum was 7cc9c60664653ddfca4d1cb016b756d054123c112116079f0a97ada510e1b07b 
but wanted 1897fd8ad4ebb523c32fabe7508c3b0b039c089661ae1e7917df0956a320ac4d
(...)

But... the checksum in

sha256 = "1897fd8ad4ebb523c32fabe7508c3b0b039c089661ae1e7917df0956a320ac4d",
is right!?

curl -sL https://download.gnome.org/sources/glib/2.77/glib-2.77.0.tar.xz | shasum -a256 -
1897fd8ad4ebb523c32fabe7508c3b0b039c089661ae1e7917df0956a320ac4d  -

Also, the checksum fails for a few jobs and the checksums these jobs get are all different o.0

So... maybe some weird temporary error?

@jjmaestro
Copy link
Contributor Author

Aaaah, I found this!

- name: Patch older branches
run: |
mkdir -p ${{ github.workspace }}/.github
curl https://raw.githubusercontent.com/bazelbuild/rules_foreign_cc/main/.github/docs-${{ matrix.ref }}.patch > ${{ github.workspace }}/.github/docs-${{ matrix.ref }}.patch
git apply ${{ github.workspace }}/.github/docs-${{ matrix.ref }}.patch
if: ${{ matrix.ref == '0.4.0' || matrix.ref == '0.3.0' || matrix.ref == '0.2.0' || matrix.ref == '0.1.0' }}

So, I guess the correct approach is to produce patches for all the old branches! A bit cumbersome but yeah, it's a good escape hatch @UebelAndre 😄

I'm going to try to patch docs/.bazelrc for those branches forcing to run without Bzlmod...

jjmaestro added a commit to jjmaestro/rules_foreign_cc that referenced this pull request Nov 6, 2024
The doc generation was failing for old releases because the .bazelrc
file wasn't disabling Bzlmod.

See:
bazel-contrib#1316 (comment)
@jjmaestro
Copy link
Contributor Author

jjmaestro commented Nov 6, 2024

OK, I think this last commit will fix the issue, but the checks fail because the curl is trying to get the patches from main and the patches don't exist for a bunch of releases that I'm now expecting to be patched :-/

The only way out I see is to create a separate PR with the patches and land it first, then rebase this PR. But that would also mean landing the PR with the patches "blind", without much testing so... maybe we could land this PR and see if it fixes the issues.

@jsharpe let me know what you want to do!

P.S. in case I need to re-do this in the future, I generated the patches with a quick-n-dirty shell script that could be easily adapted to generate other patches.

@jjmaestro
Copy link
Contributor Author

jjmaestro commented Nov 8, 2024

I've tried reproducing the checksum error with another script but I can't, the checksum of the downloaded artifact always matches the expected checksum... so I'm not sure what's happening with those first errors in the checks.

@jsharpe since the doc generation is currently broken, I don't think that landing this PR would create further issues, so... maybe we can try and see if the CI jobs finally go green? :-?

@jjmaestro
Copy link
Contributor Author

@jsharpe ping here too, I think this PR should land, the checks breaking seem completely unrelated (the weird GNOME glib checksum error that I haven't been able to reproduce).

Copy link
Member

@jsharpe jsharpe left a comment

Choose a reason for hiding this comment

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

Thank you for looking into why this was failing - I believe what you've done should work - will merge it and check!

@jsharpe jsharpe merged commit adc5080 into bazel-contrib:main Nov 18, 2024
2 of 18 checks passed
@jsharpe
Copy link
Member

jsharpe commented Nov 18, 2024

This is almost there - but think some of the patches don't quite apply e.g. 0.3.0 - github cancels all the actions as soon as one fails so its a bit annoying to check whether any of the others are also bad..

@jjmaestro
Copy link
Contributor Author

Ok, I'll give it another thought and will also try to modify the workflow to try and run all of the jobs so at least there's more info to debug!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants