Page MenuHomePhabricator

ci: trigger distcheck only if build files changed or keyword present
AbandonedPublic

Authored by zmike on Jul 23 2018, 4:46 AM.

Details

Summary

'cibuildme' , when placed in a commit log, will force
a distcheck build on CI if no build files have changed

also includes:

ci: move distcheck build skipping to earliest possible place in build

this breaks out the script to evaluate whether a distcheck should occur
into a separate file and runs it in the before_install hook, setting
a variable on skip

running configure or any commands at all is unnecessary if the distcheck
build will be skipped, so just update the last-distcheck file and abort
the build immediately to save time

ref D6735

Diff Detail

Repository
rEFL core/efl
Branch
master
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 7368
zmike created this revision.Jul 23 2018, 4:46 AM
zmike requested review of this revision.Jul 23 2018, 4:46 AM
zmike planned changes to this revision.Jul 23 2018, 5:23 AM
zmike added inline comments.
.ci/ci-make-distcheck.sh
25

This has a bug: if the previous distcheck commit is no longer in history then the whole thing fails.

zmike updated this revision to Diff 15825.Jul 23 2018, 7:37 AM
zmike edited the summary of this revision. (Show Details)

check prev_distcheck commit hash to see if it's a valid commit in current

history

bu5hm4n requested changes to this revision.Aug 3 2018, 1:18 AM

This + "only build examples in distcheck" results in the fact that examples are now only build if buildfiles are touched ? (Really not sure, thats why I ask)

This revision now requires changes to proceed.Aug 3 2018, 1:18 AM
zmike added a comment.Aug 6 2018, 4:14 AM

That's correct. At present there is little work being done related to examples (other than my own changes to build system rules), so the examples themselves aren't changing, and any substantive changes to the tree which would break examples in the short term will require build file changes.

This change is also done under the assumption also that T7182 will be accomplished immediately after the release--which I am planning to tackle myself. Some tweaks can be made at a later point to fine tune this a bit.

zmike requested review of this revision.Aug 6 2018, 4:14 AM
bu5hm4n requested changes to this revision.Aug 6 2018, 4:21 AM

A simple API change would break the examples and leave the buildfiles as they are. I am not a big fan of this, i remember @stefan_schmidt fixing examples a few times for releases, we could push that work to the committer with that, which is something that i would like to keep.

This revision now requires changes to proceed.Aug 6 2018, 4:21 AM
zmike added a comment.Aug 6 2018, 11:58 AM

Again, at this point in the release there should not be changing of APIs in a way that affects the examples build. And post release it should no longer be relevant.

I can have it check for header files as well if transferring the examples out of repo takes longer than expected.

zmike requested review of this revision.Aug 6 2018, 11:58 AM
stefan_schmidt requested changes to this revision.Aug 16 2018, 2:35 AM

Sorry, but I do not want to have such a change before the release.

This can be part of the of the split out of the examples you plan to do.

Right now I am happy to have the CI spent more time on builds just to make sure the examples are fine.

We have enough situation where this can still break (c++ disabled or enabled will have different examples build, I just pushed two patches to examples themselves, there might still be API breaks detected that need fixing)

Cutting the build time by such assumptions is not the right time just before the release. Doing it afterwards is totally different. We can, and will, have to do more experimental changes to CI.

This revision now requires changes to proceed.Aug 16 2018, 2:35 AM
zmike updated this revision to Diff 16355.Aug 16 2018, 6:26 AM
zmike edited the summary of this revision. (Show Details)

reorder/squash some patches

With the release done I am open for more experimental work like having heuristics when to build distcheck :-)

I commented on two more extensions we would need to check to make this heuristic a bit more safe.

What I can agree on is not running distcheck (especially the part where we create dist and check if all files are in CLEANFILES, etc) when build files are not touched.

The case I worry more about is the examples build. I don't think the heuristic on build files works to decide if they should build or not.

Maybe its time we move the examples build to a stage after the efl build and not inside the distcheck build?

This also depends on how the move of examples to a git submodule works out and in mid term also when we switch to meson. (the distcheck build equivalent would be very different and also faster)

There is also the option to have a Travis build triggered on a daily basis.

.ci/ci-make-distcheck.sh
22

This part came in as a separate patch by now. Needs updating.

.ci/distcheck-eval.sh
9

m4 and .in files might be also candidates for your grep here.

.travis.yml
80

why are do you using -z for the test here? Checking if the string length is zero does not make much sense. Why would you not check if the value is 1, as you set it above?

zmike abandoned this revision.Aug 21 2018, 7:34 AM

I am not sure this has much value anymore. With the full patch series, distcheck builds take around 20 minutes. I expect this to be reduced by 50-75% with a meson build. I will revisit it if this is no longer the case...