GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — RtD docs previews: Cancel building PRs if no changes in Doc dir by hugovk · Pull Request #104100 · python/cpython

Conversation

@hugovk

@hugovk hugovk commented May 2, 2023

Copy link
Copy Markdown
Member

As suggested by @humitos at #103843 (comment), cancel building pull requests when there aren't changes in the Doc directory.

If there are no changes (git diff exits with 0) we force the command to return with 183. This is a special exit code on Read the Docs that will cancel the build immediately.

https://docs.readthedocs.io/en/stable/build-customization.html#cancel-build-based-on-a-condition

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@hugovk
Cancel building PRs when there aren't changes in the Doc directory
c7a07b7
@bedevere-bot bedevere-bot added the awaiting core review label May 2, 2023
@hugovk hugovk added skip issue skip news labels May 2, 2023
@hugovk
The keys build.jobs and build.commands can't be used together
e610f22
@hugovk

hugovk commented May 2, 2023

Copy link
Copy Markdown
Member Author

For the first commit:

Problem in your project's configuration. Invalid "build.commands": .readthedocs.yml: The keys build.jobs and build.commands can't be used together.

https://readthedocs.org/projects/cpython-previews/builds/20362322/

For the second:

We encountered a problem with a command while building your project. To resolve this error, double check your project configuration and installed dependencies are correct and have not changed recently.
...
/bin/sh: 1: if: not found
/bin/sh: 2: Syntax error: "then" unexpected

https://readthedocs.org/projects/cpython-previews/builds/20362458/

@humitos Any suggestions? Thank you!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Eclips4

Eclips4 commented May 2, 2023

Copy link
Copy Markdown
Member

Just guessing, maybe move all commands and lines of bash code into jobs/post_checkout section will help?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@hugovk hugovk mentioned this pull request May 3, 2023
Replace Netlify with Read the Docs build previews #103843
Merged
3 tasks done
sobolevn
sobolevn reviewed May 12, 2023
View reviewed changes
Comment thread .readthedocs.yml Resolved

Uh oh!

There was an error while loading. Please reload this page.

@hugovk @sobolevn
Move "then" to same line as "if"
b2ec93d
Co-authored-by: Nikita Sobolev <[email protected]>
@sobolevn

sobolevn commented May 12, 2023

Copy link
Copy Markdown
Member

Hm, looks like sh cannot handle ] && expr case. Better use [ ] twice.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@hugovk
Double brackets
addeef3
sobolevn
sobolevn reviewed May 12, 2023
View reviewed changes

@sobolevn sobolevn left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Sorry, I should have posted the full version right away :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

hugovk reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread .readthedocs.yml Resolved

Uh oh!

There was an error while loading. Please reload this page.

@hugovk @sobolevn
Bracket both expressions
de18c2f
Co-authored-by: Nikita Sobolev <[email protected]>
@humitos

humitos commented May 15, 2023

Copy link
Copy Markdown
Contributor

Hi! Sorry for the late reply. I've been on vacations and then Write the Docs conference after that.

I was 100% sure that we already solved the problem you are facing, but I just found that we reverted the change because it was causing other issues (readthedocs/readthedocs.org#10206). I'm sorry for the confusion here.

As @Eclips4 commented, putting this into a script and calling it from build.jobs.post_ should work as a workaround.

hugovk reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@humitos

humitos commented May 15, 2023

Copy link
Copy Markdown
Contributor

Interesting... It seems it works fine with build.jobs (I tested it again at https://readthedocs.org/projects/test-builds/builds/20716439/) but it doesn't with build.commands for some reason. I will need to keep researching about this and I will come back, hopefully, with a solution 😄

hugovk reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sobolevn

sobolevn commented May 16, 2023

Copy link
Copy Markdown
Member

It works locally, maybe this is some kind of yaml issue?

sh-3.2$ if [ "$READTHEDOCS_VERSION_TYPE" = "external" ] && [ "$(git diff --quiet origin/main -- Doc/ .readthedocs.yml; echo $?)" -eq 0 ]; then
>   echo 1
> fi
sh-3.2$ 
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sobolevn

sobolevn commented May 16, 2023

Copy link
Copy Markdown
Member

Maybe like this?

- if [ "$READTHEDOCS_VERSION_TYPE" = "external" ] && [ "$(git diff --quiet origin/main -- Doc/ .readthedocs.yml; echo $?)" -eq 0 ]; then \
    echo "No changes to Doc/ - exiting the build."; \
    exit 183; \
  fi
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@hugovk
Adjust YAML
a32d767
@sobolevn

sobolevn commented May 16, 2023

Copy link
Copy Markdown
Member

I have no idea what is going on :)
Maybe we are missing something obvious?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@hugovk
Remove duplicate 'the'
9d2b63c
@hugovk
hugovk marked this pull request as draft May 21, 2023 20:03
@bedevere-bot bedevere-bot removed the awaiting core review label May 21, 2023
@humitos

humitos commented May 23, 2023

Copy link
Copy Markdown
Contributor

Today we are deploying a change on Read the Docs that should fix this. Hopefully, it works 🤞🏼

arhadthedev and hugovk reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@humitos

humitos commented May 23, 2023

Copy link
Copy Markdown
Contributor

The fix on Read the Docs is already deployed and my tests worked. Can you confirm that it works for you now as well? Note that you have to adapt your command to revert it to the state it was before (multi-line). I will do a suggestion on the PR.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

humitos
humitos reviewed May 23, 2023
View reviewed changes
Comment thread .readthedocs.yml Resolved

Uh oh!

There was an error while loading. Please reload this page.

arhadthedev and others added 3 commits May 23, 2023 22:37
@arhadthedev
Merge branch 'main' into rtd-doc-dir-only
d99f08f
@hugovk @humitos
Use | for multiline string
689dfe6
Co-authored-by: Manuel Kaufmann <[email protected]>
@hugovk
Fix indent
786eee1
@hugovk
hugovk marked this pull request as ready for review May 23, 2023 19:27
@bedevere-bot bedevere-bot added the awaiting core review label May 23, 2023
@hugovk
hugovk merged commit c3204ed into python:main May 24, 2023
@hugovk
hugovk deleted the rtd-doc-dir-only branch May 24, 2023 13:54
@bedevere-bot bedevere-bot removed the awaiting core review label May 24, 2023
@hugovk

hugovk commented May 27, 2023

Copy link
Copy Markdown
Member Author

This is working quite well, thanks!

One thing though, for example on #105008 (comment), the build was cancelled as expected:

Details image

https://readthedocs.org/projects/cpython-previews/builds/20835824/

However, the status check on GitHub still shows as pending:

Details image

And also shows in the tab:

Details image

Would it be possible to have it report as skipped instead of pending?

AlexWaygood and humitos reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@humitos

humitos commented May 29, 2023

Copy link
Copy Markdown
Contributor

Hi @hugovk. This should be reporting success (✅) to GitHub when the build is skipped. It seems we introduced a bug here or something that broke this behavior. If you have the time, please open an issue in https://github.com/readthedocs/readthedocs.org/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@hugovk hugovk mentioned this pull request May 29, 2023
Skipped PR build remains a pending check in GitHub UI readthedocs/readthedocs.org#10364
Closed
@hugovk

hugovk commented May 29, 2023

Copy link
Copy Markdown
Member Author

Thanks, reported: readthedocs/readthedocs.org#10364


Another PR had no Doc/ or .readthedocs.yml changes but still built the docs preview:

  • gh-89886: Rely on HAVE_SYS_TIME_H #105058
  • de000c6
  • https://readthedocs.org/projects/cpython-previews/builds/20850890/
  • https://cpython-previews--105058.org.readthedocs.build/en/105058/

We can see the check was there but returned 0:

if [ "$READTHEDOCS_VERSION_TYPE" = "external" ] && [ "$(git diff --quiet origin/main -- Doc/ .readthedocs.yml; echo $?)" -eq 0 ];
then
  echo "No changes to Doc/ - exiting the build.";
  exit 183;
fi

Any ideas why this didn't skip?

erlend-aasland reacted with eyes emoji
All reactions
  • 👀 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ezio-melotti ezio-melotti mentioned this pull request Jul 25, 2023
Remove superflous whitespaces in layout.html. #107067
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sobolevn sobolevn sobolevn left review comments
+1 more reviewer
@humitos humitos humitos left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip issue skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@hugovk @Eclips4 @sobolevn @humitos @bedevere-bot @arhadthedev
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.