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

gh-100072: only trigger netlify builds for doc changes #100074

Merged
merged 5 commits into from
Dec 7, 2022

Conversation

sobolevn
Copy link
Member

@sobolevn sobolevn commented Dec 7, 2022

This PR will trigger the netlify build because of netlify.toml change, but hopefully it won't trigger changes anymore for other builds without ./Doc or ./Misc changes.

Open question: do we need ./Misc here? All NEWS go there, many PRs have it. But, I don't think we are really interested in rendering news. Opinions?

CC @Mariatta @hugovk @epicfaace

@netlify
Copy link

netlify bot commented Dec 7, 2022

Deploy Preview for python-cpython-preview ready!

Name Link
🔨 Latest commit 35c4b75
🔍 Latest deploy log https://app.netlify.com/sites/python-cpython-preview/deploys/6390daa639c5580009d1c81d
😎 Deploy Preview https://deploy-preview-100074--python-cpython-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@hugovk
Copy link
Member

hugovk commented Dec 7, 2022

Thanks! Looks like it needs a bit of tuning.

I added this to my fork's main, which has Netlify enabled: hugovk@71faec4

Then created two PRs inside my fork, one with a Lib/ change, another with a Doc/ change:

But Netlify built both.

@sobolevn
Copy link
Member Author

sobolevn commented Dec 7, 2022

Hm, maybe like this?

@hugovk
Copy link
Member

hugovk commented Dec 7, 2022

Added that to my fork's main: hugovk@24060c3

Rebased the two PRs but both had Netlify builds.

@sobolevn
Copy link
Member Author

sobolevn commented Dec 7, 2022

This command should work:

git diff --name-only | grep -q '^Doc/' && exit 1 || exit 0

An exit code of 1 indicates the contents have changed and the build process continues as usual. An exit code of 0 indicates that there are no changes and the build should stop.

Prep:

echo 'a' >> Doc/glossary.rst

Local tests (I use echo instead of exit for demo):

» git diff --name-only | grep -q '^Lib/' && echo 1 || echo 0
0
» git diff --name-only | grep -q '^Doc/' && echo 1 || echo 0
1

Do you agree?

@hugovk
Copy link
Member

hugovk commented Dec 7, 2022

With local testing, yes.

With Netlify, no...

hugovk@abbc30d shows "Deploy Preview canceled." for both PRs...

hugovk#34 with no docs change:

7:35:00 PM: Detected ignore command in Netlify configuration file. Proceeding with the specified command: 'git diff --name-only | grep -q '^Doc/' && exit 1 || exit 0'
7:35:01 PM: User-specified ignore command returned exit code 0. Returning early from build.
7:35:01 PM: Creating deploy upload records
7:35:01 PM: Failed during stage 'checking build content for changes': Canceled build due to no content change
7:35:01 PM: Finished processing build request in 13.44133397s

hugovk#35 with docs change:

7:35:15 PM: Detected ignore command in Netlify configuration file. Proceeding with the specified command: 'git diff --name-only | grep -q '^Doc/' && exit 1 || exit 0'
7:35:16 PM: User-specified ignore command returned exit code 0. Returning early from build.
7:35:16 PM: Creating deploy upload records
7:35:17 PM: Failed during stage 'checking build content for changes': Canceled build due to no content change
7:35:17 PM: Finished processing build request in 12.800699818s

Rewinding and looking at the logs for #100074 (comment):

hugovk#34 without docs change:

4:57:39 PM: Detected ignore command in Netlify configuration file. Proceeding with the specified command: 'git diff --quiet $CACHED_COMMIT_REF $COMMIT_REF Doc/ netlify.toml'
4:57:40 PM: fatal: ambiguous argument 'Doc/': unknown revision or path not in the working tree.
4:57:40 PM: Use '--' to separate paths from revisions, like this:
4:57:40 PM: 'git <command> [<revision>...] -- [<file>...]'

hugovk#35 with docs change:

4:56:02 PM: Detected ignore command in Netlify configuration file. Proceeding with the specified command: 'git diff --quiet $CACHED_COMMIT_REF $COMMIT_REF Doc/ netlify.toml'
4:56:02 PM: fatal: ambiguous argument 'Doc/': unknown revision or path not in the working tree.
4:56:02 PM: Use '--' to separate paths from revisions, like this:
4:56:02 PM: 'git <command> [<revision>...] -- [<file>...]'

So we've already set base = "Doc/", where:

The base directory setting prompts our buildbots to change to the specified directory to detect dependencies and perform caching during the build process. It’s useful for building from a monorepo or subdirectory.

So let's try git diff --quiet $CACHED_COMMIT_REF $COMMIT_REF . ../netlify.toml: hugovk@1810da0

I also cleared the deploy caches, in case the rebase/force pushes

hugovk#34 without docs change - no Netlify build! ✅:

8:13:58 PM: Detected ignore command in Netlify configuration file. Proceeding with the specified command: 'git diff --quiet $CACHED_COMMIT_REF $COMMIT_REF . ../netlify.toml'
8:13:58 PM: User-specified ignore command returned exit code 0. Returning early from build.
8:13:58 PM: Creating deploy upload records
8:13:58 PM: Failed during stage 'checking build content for changes': Canceled build due to no content change
8:13:59 PM: Finished processing build request in 37.864234653s

hugovk#35 with docs change - has Netlify build! ✅:

8:11:51 PM: Detected ignore command in Netlify configuration file. Proceeding with the specified command: 'git diff --quiet $CACHED_COMMIT_REF $COMMIT_REF . ../netlify.toml'
8:11:51 PM: Section completed: initializing
8:11:51 PM: Starting build script
...

So let's try this!

netlify.toml Outdated Show resolved Hide resolved
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
@sobolevn
Copy link
Member Author

sobolevn commented Dec 7, 2022

Thanks, base is something I have completely missed!

Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

Thank you, let's give this a shot!

@sobolevn
Copy link
Member Author

sobolevn commented Dec 7, 2022

Thank you for testing and finding a solution ;)

@hugovk hugovk merged commit d92407e into python:main Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants