fix: "/pkgdoc/package_documentation/pkgdocs": not found
Description
This PR fixes an issue with the Docker build process that was failing with the error:
Changes Made:
- Added a new line to the Dockerfile to create the required directory structure before building the documentation.
- This ensures that the
/pkgdoc/package_documentationdirectory exists before themake pkgdocscommand runs, allowing the build process to correctly generate and store documentation inpkgdocsdirectory.
Motivation and Context
Review Time Estimate
- [ ] Immediately
- [ ] Within one week
- [ ] When possible
Types of changes
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
Checklist:
- [ ] My change requires a change to the documentation.
- [ ] My name is in the list of CITATION.cff
- [ ] I agree that PEcAn Project may distribute my contribution under any or all of
- the same license as the existing code,
- and/or the BSD 3-clause license.
- [ ] I have updated the CHANGELOG.md.
- [ ] I have updated the documentation accordingly.
- [ ] I have read the CONTRIBUTING document.
- [ ] I have added tests to cover my changes.
- [ ] All new and existing tests passed.
Is that screenshot from run CI run 40368329071 ? Hard to say why that one failed, but it looks like other runs in the develop branch, including the most recent, are succeeding. That makes me hopeful the error in 40368329071 was a transient one-off.
I also don't think what you have here would be the right fix even if the error persists -- it would copy any existing package_documentation directory in from the host system, whereas the intended design is for the Docker build to compile all pkgdown pages fresh inside the container. The package_documentation dir doesn't exist at the start of the build, but is created as part of the call to make pkgdocs.
Is that screenshot from run CI run 40368329071 ? Hard to say why that one failed, but it looks like other runs in the develop branch, including the most recent, are succeeding. That makes me hopeful the error in 40368329071 was a transient one-off.
I also don't think what you have here would be the right fix even if the error persists -- it would copy any existing
package_documentationdirectory in from the host system, whereas the intended design is for the Docker build to compile all pkgdown pages fresh inside the container. The package_documentation dir doesn't exist at the start of the build, but is created as part of the call tomake pkgdocs.
Thanks for feedback! I’ll take a closer look now.
Key improvements from the recent PR:
- Added function to generate
_pkgdown.yml: Introduced dynamic generation of_pkgdown.ymlfor all packages to customize the pkgdown site appearance. Upgraded the default Bootstrap version from 3 (deprecated) to Bootstrap 5, ensuring modern UI compatibility. Also added a persistent link to return to the main landing page (_pkgdown_docs/index.html). - Structured pkgdown output directories by source:
Refactored the _pkgdown_docs output layout by categorizing packages under their respective source directories (e.g.,
_pkgdown_docs/base/PEcAn.loggerinstead of_pkgdown_docs/PEcAn.logger). This improves clarity and reflects the actual package organization within the PEcAn source tree, rather then all the packages in the same level. - Improved automation of the main documentation landing page(
_pkgdown_docs/index.html) by grouping and linking top-level directories (base/, models/, modules/). This provides a cleaner and more navigable structure for accessing the documentation site.
Further Improvements:
While the current _pkgdown.yml setup is consistent across all packages, future improvements could include package-specific customization for finer control over individual documentation sites.
which looks like:
This commit includes:
- A button to navigate back to All Packages.
- I’ve also tried to address all the feedback mentioned above.
- Added
URLandBugReportsfields in all packageDESCRIPTIONfiles. This was done for a couple of reasons:- To avoid the warning
URLs not okwhile generating the documentation site. - Without these fields, the GitHub icon will not appear on the documentation site when using default settings.
- And to maintain consistency acorss all the packages.
- To avoid the warning
Please review and try it out. Let me know if it works for you or if any further improvements are needed
Tested the pkgdown docs build locally, and i have pushed the container to Docker registry, you can access it using these commands:
docker pull akash7022/pkg:latest
docker run -d -p 8080:80 akash7022/pkg:latest
Once the container is running, go to: http://localhost:8080
Hello sir, as we say in recent deployment to package-documentation , index.html page currently resides inside the develop/. Do you think it would be more convenient to view the documentation directly via the deployed gh page if the index.html were placed in the root (main)?
So, should we remove ${version} from rsync path of pkgdown.yml ,then the output goes directly to the root for easier access ?
Or would you suggest any alternative.
That's a good thought, but putting it in a subirectory was intentional to facilitate serving multiple versions of the docs. You only see /develop/ today, but it will eventually parallel the long-form documentation that has https://pecanproject.github.io/documentation/master/, https://pecanproject.github.io/documentation/develop/, https://pecanproject.github.io/documentation/version171/, and so on.
rsync -a --delete ../_pkgdown_docs/ ${VERSION}/
if [ "$VERSION" = "develop" ]; then
echo '<!DOCTYPE html>
<html>
<head>
<meta http-equiv="refresh" content="0; url=./develop/" />
<title>Redirecting to Documentation</title>
</head>
<body>
<p>Redirecting to <a href="./develop/">latest documentation</a>...</p>
</body>
</html>' > index.html
fi
git add --all *
so an alternative, if branch is develop , it creates a blank webpage named index.html , which immediately (0-second delay) send the visitor to the ./develop/ dir.
currently if we visit https://github.com/PecanProject/package-documentation/ we see only readme, we must manually type /develop to see docs. so in the above approach - if we visit https://github.com/PecanProject/package-documentation/ the page automatically loads https://github.com/PecanProject/package-documentation/develop/
Open to suggestions if there's a cleaner or more standard way to handle this - happy to adjust!
rsync -a --delete ../_pkgdown_docs/ ${VERSION}/ if [ "$VERSION" = "develop" ]; then echo '<!DOCTYPE html> <html> <head> <meta http-equiv="refresh" content="0; url=./develop/" /> <title>Redirecting to Documentation</title> </head> <body> <p>Redirecting to <a href="./develop/">latest documentation</a>...</p> </body> </html>' > index.html fi git add --all *so an alternative, if branch is
develop, it creates a blank webpage namedindex.html, which immediately (0-second delay) send the visitor to the./develop/dir. currently if we visithttps://github.com/PecanProject/package-documentation/we see onlyreadme, we must manually type/developto see docs. so in the above approach - if we visithttps://github.com/PecanProject/package-documentation/the page automatically loadshttps://github.com/PecanProject/package-documentation/develop/Open to suggestions if there's a cleaner or more standard way to handle this - happy to adjust!
hello sir, any insights on this approach?
@robkooper Thanks for your feedback, and sorry for the delay, since it's a busy week with assessments, so i couldn't address this sooner
looks like there is a hardcoded devlop, see comment.
this is a good catch! ,this should be made dynamic to detect the current branch. We could make the configuration like:
branch <- Sys.getenv("PECAN_GIT_BRANCH", unset = "develop")
source = paste0("https://github.com/PecanProject/pecan/blob/", branch, "/", pkg)
in the build_pkgdown.R
And in the GHA workflow (pkgdown.yml), we could add :
env:
PECAN_GIT_BRANCH: ${{ github.event_name == 'pull_request' && github.base_ref || github.ref_name }}
The github.base_ref gives the target branch name during the PR, but if the R script is run on a push requests and other events types, github.ref_name will provide the branch or tag name.
This approach ensures that the branch name is always correctly set to the target branch for PRs and the branch name for push request and other groups, which is exactly what we want for generating correct links in the documentation.
Let me know if you have any suggestions or would like me to proceed with these changes.
rsync -a --delete ../_pkgdown_docs/ ${VERSION}/ if [ "$VERSION" = "develop" ]; then echo '<!DOCTYPE html> <html> <head> <meta http-equiv="refresh" content="0; url=./develop/" /> <title>Redirecting to Documentation</title> </head> <body> <p>Redirecting to <a href="./develop/">latest documentation</a>...</p> </body> </html>' > index.html fi git add --all *so an alternative, if branch is
develop, it creates a blank webpage namedindex.html, which immediately (0-second delay) send the visitor to the./develop/dir. currently if we visithttps://github.com/PecanProject/package-documentation/we see onlyreadme, we must manually type/developto see docs. so in the above approach - if we visithttps://github.com/PecanProject/package-documentation/the page automatically loadshttps://github.com/PecanProject/package-documentation/develop/Open to suggestions if there's a cleaner or more standard way to handle this - happy to adjust!
@robkooper If you have a moment, could you please take a look at this following conversion ?
I'm sorry for my long delay answering here.
currently if we visit https://github.com/PecanProject/package-documentation/ we see only readme, we must manually type /develop to see docs
I think what we want to see at that URL is a list of available versions, like we currently see when we visit https://pecanproject.github.io/documentation. Deciding how to implement that can be a separate PR -- for this one it seems fine to leave that as it is and remember to give people links that have the /develop/ already included.
@divine7022 what are next steps here?
@dlebauer Apologies for taking so long to respond here.
I think this is ready to merge as-is;
It addresses the Bootstrap 3 deprecation warnings we've been getting by upgrading to Bootstrap 5, plus adds some nice navigation improvements with the custom back button, dynamic branch-specific links to source code of a package and BugReports, and some other improvements.
As @infotroph mentioned, we can have a separate PR for the landing page version-specific links listed.