Re: Build the docs if there are changes in docs and don't run other tasks if the changes are only in docs
Hi, On Sun, 12 May 2024 at 14:53, Peter Eisentraut wrote: > > On 14.12.23 14:40, Nazir Bilal Yavuz wrote: > > On Fri, 6 Oct 2023 at 17:07, Tom Lane wrote: > >> > >> As a quick cross-check, I searched our commit log to see how many > >> README-only commits there were so far this year. I found 11 since > >> January. (Several were triggered by the latest round of pgindent > >> code and process changes, so maybe this is more than typical.) > >> > >> Not sure what that tells us about the value of changing the CI > >> logic, but it does seem like it could be worth the one-liner > >> change needed to teach buildfarm animals to ignore READMEs. > > > > I agree that it could be worth implementing this logic on buildfarm animals. > > > > In case we want to implement the same logic on the CI, I added a new > > version of the patch; it skips CI completely if the changes are only > > in the README files. > > I don't see how this could be applicable widely enough to be useful: > > - While there are some patches that touch on README files, very few of > those end up in a commit fest. > > - If someone manually pushes a change to their own CI environment, I > don't see why we need to second-guess that. > > - Buildfarm runs generally batch several commits together, so it is very > unlikely that this would be hit. > > I think unless some concrete reason for this change can be shown, we > should drop it. These points make sense. I thought that it is useful regarding Tom's '11 README-only commit since January' analysis (at 6 Oct 2023) but this may not be enough on its own. If there are no objections, I will withdraw this soon. -- Regards, Nazir Bilal Yavuz Microsoft
Re: Build the docs if there are changes in docs and don't run other tasks if the changes are only in docs
On 14.12.23 14:40, Nazir Bilal Yavuz wrote: On Fri, 6 Oct 2023 at 17:07, Tom Lane wrote: As a quick cross-check, I searched our commit log to see how many README-only commits there were so far this year. I found 11 since January. (Several were triggered by the latest round of pgindent code and process changes, so maybe this is more than typical.) Not sure what that tells us about the value of changing the CI logic, but it does seem like it could be worth the one-liner change needed to teach buildfarm animals to ignore READMEs. I agree that it could be worth implementing this logic on buildfarm animals. In case we want to implement the same logic on the CI, I added a new version of the patch; it skips CI completely if the changes are only in the README files. I don't see how this could be applicable widely enough to be useful: - While there are some patches that touch on README files, very few of those end up in a commit fest. - If someone manually pushes a change to their own CI environment, I don't see why we need to second-guess that. - Buildfarm runs generally batch several commits together, so it is very unlikely that this would be hit. I think unless some concrete reason for this change can be shown, we should drop it.
Re: Build the docs if there are changes in docs and don't run other tasks if the changes are only in docs
On 2023-10-06 Fr 10:07, Tom Lane wrote: Peter Eisentraut writes: I don't have a good sense of what you are trying to optimize for. If it's the mainline build-on-every-commit type, then I wonder how many commits would really be affected by this. Like, how many commits touch only a README file. If it's for things like the cfbot, then I think the time-triggered builds would be more frequent than new patch versions, so I don't know if these kinds of optimizations would affect anything. As a quick cross-check, I searched our commit log to see how many README-only commits there were so far this year. I found 11 since January. (Several were triggered by the latest round of pgindent code and process changes, so maybe this is more than typical.) Not sure what that tells us about the value of changing the CI logic, but it does seem like it could be worth the one-liner change needed to teach buildfarm animals to ignore READMEs. - trigger_exclude => qr[^doc/|\.po$], + trigger_exclude => qr[^doc/|/README$|\.po$], I've put that in the sample config file for the next release. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Re: Build the docs if there are changes in docs and don't run other tasks if the changes are only in docs
> On 14 Dec 2023, at 14:40, Nazir Bilal Yavuz wrote: > On Fri, 6 Oct 2023 at 17:07, Tom Lane wrote: >> Not sure what that tells us about the value of changing the CI >> logic, but it does seem like it could be worth the one-liner >> change needed to teach buildfarm animals to ignore READMEs. > > I agree that it could be worth implementing this logic on buildfarm animals. > > In case we want to implement the same logic on the CI, I added a new > version of the patch; it skips CI completely if the changes are only > in the README files. I think it makes sense to avoid wasting CI cycles on commits only changing README files since we know it will be a no-op. A README documentation patch going through N revisions will incur at least N full CI runs which are resources we can spend on other things. So +1 for doing this both in CI and in the buildfarm. -- Daniel Gustafsson
Re: Build the docs if there are changes in docs and don't run other tasks if the changes are only in docs
Hi,
Sorry for the late reply.
On Fri, 6 Oct 2023 at 17:07, Tom Lane wrote:
>
> As a quick cross-check, I searched our commit log to see how many
> README-only commits there were so far this year. I found 11 since
> January. (Several were triggered by the latest round of pgindent
> code and process changes, so maybe this is more than typical.)
>
> Not sure what that tells us about the value of changing the CI
> logic, but it does seem like it could be worth the one-liner
> change needed to teach buildfarm animals to ignore READMEs.
I agree that it could be worth implementing this logic on buildfarm animals.
In case we want to implement the same logic on the CI, I added a new
version of the patch; it skips CI completely if the changes are only
in the README files.
--
Regards,
Nazir Bilal Yavuz
Microsoft
From 6c268233d13262a31965baf2dbb42913d83dab1d Mon Sep 17 00:00:00 2001
From: Nazir Bilal Yavuz
Date: Thu, 14 Dec 2023 16:23:28 +0300
Subject: [PATCH v3] If the changes are only in the README files, skip all the
tasks.
If the changes are only in the README files, skip all the tasks to save
CI time.
---
.cirrus.tasks.yml | 17 -
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index e137769850..dfd12f8b04 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -4,6 +4,12 @@
# further details, see src/tools/ci/README
+# If the changes are only in the README files, skip all the tasks.
+#
+# This skip is overriden in 'SanityCheck' task. So, the same check is
+# added 'SanityCheck' task too.
+skip: changesIncludeOnly('**/README')
+
env:
# The lower depth accelerates git clone. Use a bit of depth so that
# concurrent tasks and retrying older jobs have a chance of working.
@@ -55,11 +61,12 @@ on_failure_meson: _failure_meson
task:
name: SanityCheck
- # If a specific OS is requested, don't run the sanity check. This shortens
- # push-wait-for-ci cycle time a bit when debugging operating system specific
- # failures. Uses skip instead of only_if, as cirrus otherwise warns about
- # only_if conditions not matching.
- skip: $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:.*'
+ # Don't run the sanity check if the changes are only in the README files or
+ # if a specific OS is requested. The latter shortens push-wait-for-ci cycle
+ # time a bit when debugging operating system specific failures. Uses skip
+ # instead of only_if, as cirrus otherwise warns about only_if conditions not
+ # matching.
+ skip: $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:.*' || changesIncludeOnly('**/README')
env:
CPUS: 4
--
2.43.0
Re: Build the docs if there are changes in docs and don't run other tasks if the changes are only in docs
Peter Eisentraut writes: > I don't have a good sense of what you are trying to optimize for. If > it's the mainline build-on-every-commit type, then I wonder how many > commits would really be affected by this. Like, how many commits touch > only a README file. If it's for things like the cfbot, then I think the > time-triggered builds would be more frequent than new patch versions, so > I don't know if these kinds of optimizations would affect anything. As a quick cross-check, I searched our commit log to see how many README-only commits there were so far this year. I found 11 since January. (Several were triggered by the latest round of pgindent code and process changes, so maybe this is more than typical.) Not sure what that tells us about the value of changing the CI logic, but it does seem like it could be worth the one-liner change needed to teach buildfarm animals to ignore READMEs. - trigger_exclude => qr[^doc/|\.po$], + trigger_exclude => qr[^doc/|/README$|\.po$], regards, tom lane
Re: Build the docs if there are changes in docs and don't run other tasks if the changes are only in docs
On 26.09.23 16:51, Nazir Bilal Yavuz wrote: Also note that there are also dependencies in the other direction. For example, the psql help is compiled from XML DocBook sources. So your other patch would also need to include similar changesInclude() clauses. If there are more cases like this, it may not be worth it. Instead, we can just: - Build the docs when the doc related files are changed (This still creates a dependency like you said). - Skip CI completely if the README files are changed. What are your opinions on these? I don't have a good sense of what you are trying to optimize for. If it's the mainline build-on-every-commit type, then I wonder how many commits would really be affected by this. Like, how many commits touch only a README file. If it's for things like the cfbot, then I think the time-triggered builds would be more frequent than new patch versions, so I don't know if these kinds of optimizations would affect anything.
Re: Build the docs if there are changes in docs and don't run other tasks if the changes are only in docs
Hi,
On Tue, 26 Sept 2023 at 13:48, Peter Eisentraut wrote:
>
> On 25.09.23 12:56, Nazir Bilal Yavuz wrote:
> > + # Only run if a specific OS is not requested and if there are changes in
> > docs
> > + # or in the CI files.
> > + skip: >
> > +$CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:.*' ||
> > +!changesInclude('doc/**',
> > +'.cirrus.yml',
> > +'.cirrus.tasks.yml',
> > +'src/backend/catalog/sql_feature_packages.txt',
> > +'src/backend/catalog/sql_features.txt',
> > +'src/backend/utils/errcodes.txt',
> > +'src/backend/utils/activity/wait_event_names.txt',
> > +
> > 'src/backend/utils/activity/generate-wait_event_types.pl',
> > +'src/include/parser/kwlist.h')
>
> This is kind of annoying. Now we need to maintain yet another list of
> these dependencies and keep it in sync with the build systems.
I agree.
>
> I think meson can produce a dependency tree from a build. Maybe we
> could use that somehow and have Cirrus cache it between runs?
I will check that.
>
> Also note that there are also dependencies in the other direction. For
> example, the psql help is compiled from XML DocBook sources. So your
> other patch would also need to include similar changesInclude() clauses.
>
If there are more cases like this, it may not be worth it. Instead, we can just:
- Build the docs when the doc related files are changed (This still
creates a dependency like you said).
- Skip CI completely if the README files are changed.
What are your opinions on these?
Regards,
Nazir Bilal Yavuz
Microsoft
Re: Build the docs if there are changes in docs and don't run other tasks if the changes are only in docs
On 25.09.23 12:56, Nazir Bilal Yavuz wrote:
+ # Only run if a specific OS is not requested and if there are changes in docs
+ # or in the CI files.
+ skip: >
+$CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:.*' ||
+!changesInclude('doc/**',
+'.cirrus.yml',
+'.cirrus.tasks.yml',
+'src/backend/catalog/sql_feature_packages.txt',
+'src/backend/catalog/sql_features.txt',
+'src/backend/utils/errcodes.txt',
+'src/backend/utils/activity/wait_event_names.txt',
+'src/backend/utils/activity/generate-wait_event_types.pl',
+'src/include/parser/kwlist.h')
This is kind of annoying. Now we need to maintain yet another list of
these dependencies and keep it in sync with the build systems.
I think meson can produce a dependency tree from a build. Maybe we
could use that somehow and have Cirrus cache it between runs?
Also note that there are also dependencies in the other direction. For
example, the psql help is compiled from XML DocBook sources. So your
other patch would also need to include similar changesInclude() clauses.
Re: Build the docs if there are changes in docs and don't run other tasks if the changes are only in docs
Hi,
I attached the second version of the patch.
On Mon, 11 Sept 2023 at 15:11, Daniel Gustafsson wrote:
>
> > On 11 Sep 2023, at 13:03, Nazir Bilal Yavuz wrote:
>
> >> Almost, but not entirely. There are a set of scripts which generate
> >> content
> >> for the docs based on files in src/, like
> >> src/backend/catalog/sql_features.txt
> >> and src/include/parser/kwlist.h. If those source files change, or their
> >> scripts, it would be helpful to build docs.
> >
> > Thanks! Are these the only files that are not in the doc subfolders
> > but effect docs?
>
> I believe so, the list of scripts and input files can be teased out of the
> doc/src/sgml/meson.build file.
Now the files mentioned in the doc/src/sgml/meson.build file will
trigger building the docs task. Also, if the changes are only in the
README files, CI will be skipped completely.
Any kind of feedback would be appreciated.
Regards,
Nazir Bilal Yavuz
Microsoft
From 843d0c132c0f95dfee50eaaf667a92fffe400eeb Mon Sep 17 00:00:00 2001
From: Nazir Bilal Yavuz
Date: Mon, 11 Sep 2023 13:29:33 +0300
Subject: [PATCH v2 2/2] Skip CI if changes are only in docs or in the READMEs
When changes are only in docs or in the READMEs, skip all the tasks.
This skip is overriden in 'SanityCheck' and 'Build the Docs' tasks. So,
these two tasks might have not been skipped.
---
.cirrus.tasks.yml | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index d7c45224af9..ec6bcd5af70 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -3,6 +3,11 @@
# For instructions on how to enable the CI integration in a repository and
# further details, see src/tools/ci/README
+# When changes are only in docs or in the READMEs, skip all the tasks.
+#
+# This skip is overriden in 'SanityCheck' and 'Build the Docs' tasks. So,
+# these two tasks might have not been skipped.
+skip: changesIncludeOnly('doc/**', '**/README')
env:
# The lower depth accelerates git clone. Use a bit of depth so that
@@ -54,12 +59,12 @@ on_failure_meson: _failure_meson
# broken commits, have a minimal task that all others depend on.
task:
name: SanityCheck
-
- # If a specific OS is requested, don't run the sanity check. This shortens
+ # If a specific OS is requested or if there are changes only in the docs
+ # or in the READMEs, don't run the sanity check. This shortens
# push-wait-for-ci cycle time a bit when debugging operating system specific
# failures. Uses skip instead of only_if, as cirrus otherwise warns about
# only_if conditions not matching.
- skip: $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:.*'
+ skip: $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:.*' || changesIncludeOnly('doc/**', '**/README')
env:
CPUS: 4
--
2.40.1
From 31fcd0c5b588e9c4ef968e7a92489cfb86740228 Mon Sep 17 00:00:00 2001
From: Nazir Bilal Yavuz
Date: Thu, 7 Sep 2023 17:58:06 +0300
Subject: [PATCH v2 1/2] Only built the docs if there are changes are in the
docs
Building the docs triggered although there are no changes in the docs.
So, the new 'Building the Docs' task is created. This task only
run if a specific OS is not requested and if there are changes in docs
or in the CI files.
---
.cirrus.tasks.yml | 77 ++-
1 file changed, 62 insertions(+), 15 deletions(-)
diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index e137769850d..d7c45224af9 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -740,21 +740,6 @@ task:
make -s -j${BUILD_JOBS} clean
time make -s -j${BUILD_JOBS} world-bin
- ###
- # Verify docs can be built
- ###
- # XXX: Only do this if there have been changes in doc/ since last build
- always:
-docs_build_script: |
- time ./configure \
---cache gcc.cache \
-CC="ccache gcc" \
-CXX="ccache g++" \
-CLANG="ccache clang"
- make -s -j${BUILD_JOBS} clean
- time make -s -j${BUILD_JOBS} -C doc
-
- ###
# Verify headerscheck / cpluspluscheck succeed
#
# - Don't use ccache, the files are uncacheable, polluting ccache's
@@ -777,3 +762,65 @@ task:
always:
upload_caches: ccache
+
+
+task:
+ name: Build the Docs
+ depends_on: SanityCheck
+ # Only run if a specific OS is not requested and if there are changes in docs
+ # or in the CI files.
+ skip: >
+$CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:.*' ||
+!changesInclude('doc/**',
+'.cirrus.yml',
+'.cirrus.tasks.yml',
+'src/backend/catalog/sql_feature_packages.txt',
+'src/backend/catalog/sql_features.txt',
+'src/backend/utils/errcodes.txt',
+'src/backend/utils/activity/wait_event_names.txt',
+'src/backend/utils/activity/generate-wait_event_types.pl',
+'src/include/parser/kwlist.h')
+
+ env:
+CPUS: 4
+BUILD_JOBS: 4
+IMAGE_FAMILY: pg-ci-bullseye
+
Re: Build the docs if there are changes in docs and don't run other tasks if the changes are only in docs
> On 11 Sep 2023, at 13:03, Nazir Bilal Yavuz wrote: >> Almost, but not entirely. There are a set of scripts which generate content >> for the docs based on files in src/, like >> src/backend/catalog/sql_features.txt >> and src/include/parser/kwlist.h. If those source files change, or their >> scripts, it would be helpful to build docs. > > Thanks! Are these the only files that are not in the doc subfolders > but effect docs? I believe so, the list of scripts and input files can be teased out of the doc/src/sgml/meson.build file. -- Daniel Gustafsson
Re: Build the docs if there are changes in docs and don't run other tasks if the changes are only in docs
Hi, Thanks for the reply! On Fri, 8 Sept 2023 at 11:05, Daniel Gustafsson wrote: > > > On 7 Sep 2023, at 18:06, Nazir Bilal Yavuz wrote: > > > if the changes are only in the docs, don't run > > all tasks except building the docs task; this could help to save more > > CI times. > > A related idea for docs in order to save CI time: if the changes are only in > internal docs, ie README files, then don't run any tasks at all. Looking at > src/backend/parser/README the last two commits only touched that file, and > while such patches might not be all that common, spending precious CI > resources > on them seems silly if we can avoid it. > > It doesn't have to be included in this, just wanted to bring it up as it's > related. I liked the idea, I am planning to edit the 0002 patch. CI won't run any tasks if the changes are only in the README files. > Almost, but not entirely. There are a set of scripts which generate content > for the docs based on files in src/, like src/backend/catalog/sql_features.txt > and src/include/parser/kwlist.h. If those source files change, or their > scripts, it would be helpful to build docs. Thanks! Are these the only files that are not in the doc subfolders but effect docs? Regards, Nazir Bilal Yavuz Microsoft
Re: Build the docs if there are changes in docs and don't run other tasks if the changes are only in docs
> On 7 Sep 2023, at 18:06, Nazir Bilal Yavuz wrote: > if the changes are only in the docs, don't run > all tasks except building the docs task; this could help to save more > CI times. A related idea for docs in order to save CI time: if the changes are only in internal docs, ie README files, then don't run any tasks at all. Looking at src/backend/parser/README the last two commits only touched that file, and while such patches might not be all that common, spending precious CI resources on them seems silly if we can avoid it. It doesn't have to be included in this, just wanted to bring it up as it's related. > I attached two patches. > > I assumed that the docs related changes are limited with the changes > in the docs folder but I am not really sure about that. Almost, but not entirely. There are a set of scripts which generate content for the docs based on files in src/, like src/backend/catalog/sql_features.txt and src/include/parser/kwlist.h. If those source files change, or their scripts, it would be helpful to build docs. -- Daniel Gustafsson
