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

Site preview build and xref validation not correctly done when the PR is created from a fork repository #402

Closed
7 tasks done
tbouffard opened this issue Aug 16, 2022 · 4 comments
Assignees
Labels
bug Something isn't working CI ⚙️

Comments

@tbouffard
Copy link
Member

tbouffard commented Aug 16, 2022

ℹ️ Part of a top-level initiative: #670

This causes issue when using the new doc-site surge-preview action (developed in #378) for #239 and #270

Previously, we didn't build the preview in such case. Since we build it (but don't deploy it), we see new errors: the branch of the PR is not part of the site build.

[UPDATE 2024-03-27]
The xref validation fails as some other components need it.
Detected in bonitasoft/bonita-doc#2058 (comment)

The xref validation is not done as well, the branch of the PR is no available as well (use the same commands as to build the preview)

[10:13:57.980] INFO (@antora/content-aggregator): No matching references found for content source entry (url: https://github.com/bonitasoft/bonita-cloud-doc.git | branches: [patch-1])
[10:13:57.983] INFO (bonita-log-aggregated-component): Discovered the following component versions
[10:13:58.592] INFO (bonita-versions-sorter): Start versions sorting

I have started to fix the issue in bonitasoft/bonita-doc#2108: it targets "2021.1" which only build the PR branch and no extra component.

The current build preview command is

./build-preview.bash --branch "patch-1" --component "bonita" \
    --pr "2108" --site-url "https://bonitasoft-bonita-doc-build_preview-pr-2108.surge.sh"

There is no build error, but the generated site is empty and there is no warning.

Building preview...
[08:35:20.963] WARN (@antora/content-classifier): Start page specified for site not found: bonita::index.adoc
[08:35:20.967] INFO (bonita-versions-sorter): Start sorting
[08:35:20.967] INFO (bonita-versions-sorter): Sort done
Preview built

Root cause

The branch of the fork repository is not available in the repository under the name it has been created in the fork repository ("patch-1" in our example).
So when Antora builds, it tries to checkout it, and as it is not available, ignores it.

Attempt to fix

Currently, we compute the PR branch with echo "BRANCH_NAME=$(echo ${GITHUB_HEAD_REF#refs/heads/})" >> $GITHUB_ENV

What we could try

  • GITHUB_REF or github.ref (refs/pull/<pr_number>/merge)

Already tested and does not work

  • GITHUB_HEAD_REF or github.head_ref: patch-1
  • GITHUB_REF_NAME or github.ref_name: 2108/merge
  • manually created pull/${{ github.event.pull_request.number }}/head
./build-preview.bash --branch "pull/2108/head" --component "bonita" \
    --pr "2108" --site-url "https://bonitasoft-bonita-doc-build_preview-pr-2108.surge.sh"

Resources

Implementation tasks

@tbouffard
Copy link
Member Author

⚠️ As described in https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally#modifying-an-inactive-pull-request-locally, the remote branch from the forked repository available in the under the pull/ID/head name in the "target/main" repository must be fetched manually with git fetch origin pull/ID/head:BRANCH_NAME.
It is not fetched with the regular git fetch command that is run by Antora. So Antora cannot access it. We must find another way to retrieve it.

@tbouffard
Copy link
Member Author

tbouffard commented Nov 17, 2022

ℹ️ Here is my proposals based on my investigations

Build preview

When building the preview for fork repositories, we need to use the URL of the fork repository. This is not possible today with the preview script, as we use the component name and the script automatically selects the repository URL.
We need to introduce a new option that will allow to set the repository URL. It will be used to build the Antora playbook only when building a single component version/branch.

Using the dedicated URL when building the preview also has an additional advantage. It correctly set the link in the built preview for page edits. It will help the contribution workflow: download the preview archive, browse it locally, find a typo, click on the "edit this page link", update on GitHub UI, commit, check the newly built preview.

This new option may also allow to build the preview of a new component under development prior it is registered in the build preview script or for a poc without having to update the script logic.

Build Site (used to validate reference)

🚧 [UPDATE 2024-02-16] Atlas is now in place, so updating the build-site action (which does xref validation) is now quite similar to what has to be done in the build-preview action.

⚠️ I propose to not change the "build site" logic to support PR from fork repositories
Implementing it would require to update the received "build preview" command
We would need to have the name of the component to update (this requires to change all workflows calling the action), remove the current branch from the list of branches related to this component, set the url of the forked repository and use the branch of this repository.

This is quite a lot of work.
Remember that we build the site as a temporar solution until we can validate all xref when building a single component version. This is a work in progress and I would prefer to invest time on using Antora Atlas instead, see #326.
So, I suggest we only skip the build when the PR is created from a fork repository.

Tasks

preview

  • add a new option in the preview script to pass a repo url, for instance component-repo-url
    • only used when building a single component branch/version (update the error message)
    • the component name should also be passed in this case for eventual usage for other features
  • in the shared "surge preview" action, detect if the PR is built from a fork, if so pass the fork repo url the "build preview" script. Otherwise,
    • Note that the pr-title-cc action in bonitasoft/actions contains the logic to detect when a PR is built from a work.
    • The logic may be reused or factorized in a new action if needed

As part of the change, we could rename the "ignore-errors" (it currently misses a trailing s) of the "build-preview" script

build-site
Apply the same kind of changes as in the "preview" actions.
If needed, introduce shared actions to share the logic between "build-site" and "preview".

@benjaminParisel
Copy link
Contributor

This week, we got a contribution from a forked repository on Bonita-doc. I download logs of two actions: Publish Preview and Validate Xref.

build_publish_preview.zip
validate_xref_from_fork.zip

@tbouffard tbouffard changed the title Site preview build fails when the PR is created from a fork repository Site preview build and xref validation not correctly done when the PR is created from a fork repository Mar 27, 2024
@tbouffard tbouffard self-assigned this Apr 16, 2024
benjaminParisel pushed a commit that referenced this issue Apr 17, 2024
…698)

Let pass the git repo url to the preview script:
  - Only valid for a branch preview for a component.
- This allows you to use an alternative URL to the one configured for
the component, in particular to use a fork.

The "build-and-publish-pr-preview" and the "build-pr-site" (used for
references validation) actions set this new argument to use the git URL
of the branch of the PR
This ensures that the git URL of the repository whose the PR is
originated from is used (fork or upstream repository).

Covers #402
benjaminParisel pushed a commit to bonitasoft/bonita-doc that referenced this issue Apr 17, 2024
This applies to the "build preview" and the "references validation".

The content of the branch of the fork is now correctly used. Previously, the branch of the fork wasn't found by Antora, so the content of the generated site was empty.

### Notes

Covers
bonitasoft/bonita-documentation-site#402
This depends on
bonitasoft/bonita-documentation-site#698
@tbouffard tbouffard moved this from In Progress to In Review in Bonita documentation site - work in progress Apr 18, 2024
benjaminParisel pushed a commit to bonitasoft/bonita-continuous-delivery-doc that referenced this issue Apr 18, 2024
"contribution checks"
The workflow now runs on `pull_request_target` events.
There are no security issues here. Checks are made only on the updated
PR file without doing any tool installation, cache update or branch
check. Only the GitHub API is used.
Using this event allows you to create a PR comment when the PR is
created from a forked repository.

"build preview" and "references validation" workflows.
The content of the branch of the fork is now correctly used. Previously,
the branch of the fork wasn't found by Antora, so the content of the
generated site was empty.

### Notes

Covers
bonitasoft/bonita-documentation-site#402
Covers
bonitasoft/bonita-documentation-site#685
benjaminParisel pushed a commit to bonitasoft/bonita-central-doc that referenced this issue Apr 18, 2024
"contribution checks"
The workflow now runs on `pull_request_target` events.
There are no security issues here. Checks are made only on the updated
PR file without doing any tool installation, cache update or branch
check. Only the GitHub API is used.
Using this event allows you to create a PR comment when the PR is
created from a forked repository.

"build preview" and "references validation" workflows.
The content of the branch of the fork is now correctly used. Previously,
the branch of the fork wasn't found by Antora, so the content of the
generated site was empty.

### Notes

Covers
bonitasoft/bonita-documentation-site#402
Covers
bonitasoft/bonita-documentation-site#685
benjaminParisel pushed a commit to bonitasoft/bonita-cloud-doc that referenced this issue Apr 18, 2024
"contribution checks"
The workflow now runs on `pull_request_target` events.
There are no security issues here. Checks are made only on the updated
PR file without doing any tool installation, cache update or branch
check. Only the GitHub API is used.
Using this event allows you to create a PR comment when the PR is
created from a forked repository.

"build preview" and "references validation" workflows.
The content of the branch of the fork is now correctly used. Previously,
the branch of the fork wasn't found by Antora, so the content of the
generated site was empty.

### Notes

Covers
bonitasoft/bonita-documentation-site#402
Covers
bonitasoft/bonita-documentation-site#685
rbioteau pushed a commit to bonitasoft/bonita-test-toolkit-doc that referenced this issue Apr 18, 2024
"contribution checks"
The workflow now runs on `pull_request_target` events.
There are no security issues here. Checks are made only on the updated
PR file without doing any tool installation, cache update or branch
check. Only the GitHub API is used.
Using this event allows you to create a PR comment when the PR is
created from a forked repository.

"build preview" and "references validation" workflows.
The content of the branch of the fork is now correctly used. Previously,
the branch of the fork wasn't found by Antora, so the content of the
generated site was empty.

### Notes

Covers
bonitasoft/bonita-documentation-site#402
Covers
bonitasoft/bonita-documentation-site#685
@tbouffard
Copy link
Member Author

All tasks are completed, so closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working CI ⚙️
Development

No branches or pull requests

2 participants