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

Do open tickets about broken links #1960

Open
wants to merge 4 commits into
base: gh-pages
Choose a base branch
from

Conversation

dscho
Copy link
Member

@dscho dscho commented Feb 27, 2025

Changes

  • This fixes the issue where the deploy workflow code wants to open a ticket in case of any broken link, but due to a bug would fail to do so.
  • This PR also increases the coverage so that links that point back to this site through "https://git-scm.com/..." links are also checked in PR builds and deployments in forks for broken links (previously, only relative links, or absolute links to the fork's GitHub Pages would have been checked, but there are plenty of links in "/book/" and in "/docs/" that were thusly missed).

Context

I was quite surprised that the most recent deployment worked but all of a sudden complained about a broken link. This was uncovered by #1953, because we now asked lychee to verify "https://git-scm.com/" links as part of the deploy workflow (but not in forks, and neither in the ci workflow that powers PR builds where this issue would ideally have been caught). Even more surprising was the fact that this broken link was not reported in a new GitHub issue, even if that was intended.

So here is some code that will hopefully prevent similar surprises in the future.

There is specific code in git-scm.com's deploy workflow to open a ticket
whenever broken links were detected, and to close such a ticket when no
broken links were detected.

However, as per lycheeverse/lychee-action#265
the way we checked for this was incorrect: `env.lychee_exit_code` had
was correct, until lycheeverse/lychee-action#245
broke it by way of fixing another bug.

This was the reason why the broken link that was found and reported in
https://github.com/git/git-scm.com/actions/runs/13544529063/job/37852881418#step:3:135319
never made it into a GitHub issue, even if that had been the intention.

For the record, I worked on a fix for this broken link and opened
jnavila/git-manpages-l10n#131 to incorporate
that fix.

What prevented this broken link from being detected before
above-mentioned deployment is the fact that before
git#1953 was merged, the
`git-scm.com` deployment used `http://` in the base URL, and hence the
`--remap` option used in the deployment workflow mapped all
http://git-scm.com links to local files, whereas now the `https://` base
URL is used and https://git-scm.com links are mapped, including the
offending one.

In any case let's fix opening/closing the "broken link" issues.

Signed-off-by: Johannes Schindelin <[email protected]>
Especially the first one, which typically starts with
`(https://<username>.github.io/)?/git-scm.com)`, needs to be anchored to
the beginning of the link, otherwise `/git-scm.com` would be matched
even in the middle of any link, which was not intended (because the
replacement starts with `file:/` and hence must be anchored at the
start.

Signed-off-by: Johannes Schindelin <[email protected]>
When deploying in forks, the base URL is distinctly different from
https://git-scm.com. But there are plenty of such links in the HTML
pages, e.g. all of those links in the ProGit book that point to Git's
homepage. Let's include those in the checks so that broken links are
noticed earlier rather than later.

Signed-off-by: Johannes Schindelin <[email protected]>
@dscho dscho self-assigned this Feb 27, 2025
Not all links are relative in the PR build; Some of the links originate
from Git's manual pages or from the ProGit book or its translations, and
those links are of the form "https://git-scm.com/...". Let's map those
into "file:///..." URLs so that lychee checks them even in offline mode.

This uncovers a broken link which is address by
jnavila/git-manpages-l10n#131. For the time
being, this patch adds a work-around specifically for that issue lest
contributors' PRs fail through no fault of their own.

Signed-off-by: Johannes Schindelin <[email protected]>
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.

1 participant