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

Make use of rsync optional in build.sh #307

Closed
wants to merge 1 commit into from

Conversation

ssbarnea
Copy link
Member

Fixes: #306

Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

The whole reason of using rsync with the temp directory is that Sphinx only rebuilds the changed files in subsequent runs, and that files that should no longer be there are deleted. You cannot simply replace rsync by cp here without breaking both these things.

Instead of doing some silent fallback, I would implement this differently so that the user can pass a flag to antsibull-docs sphinx-init that directly lets antsibull-docs write to the destination instead of writing into a temp directory and using rsync. You can then use that option in CI when you start with a clean state anyway and don't need incremental builds.

@felixfontein
Copy link
Collaborator

(To re-create the sphinx-init baseline, go to tests/functional/ and run ./build-sphinx-init-baseline.sh.)

@@ -4,12 +4,26 @@
# SPDX-License-Identifier: GPL-3.0-or-later

# Created with antsibull-docs @{ antsibull_docs_version }@
# cspell: disable cprv
Copy link
Collaborator

Choose a reason for hiding this comment

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

We don't use cspell in this repository (whether or not we should is a separate discussion), so it'd be better not to have comments for it cluttering up the sources.

@felixfontein
Copy link
Collaborator

I'm planning to do a new feature release of antsibull-docs this weekend. This would be a nice feature to include; when do you plan to continue working on it?

@felixfontein
Copy link
Collaborator

#315 implements this differently.

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.

Use of rsync inside generated build.sh prevents usage with readthedocs
3 participants