Appveyor and Travis should use "make -k"

Right now our .appveyor.yml and .travis.yml are not using the "-k" make flag. That means that once it encounters an error, it stops building. In general, we would prefer our CI to keep trying to build for a long as it can, so that we can see all of the warnings and errors that are produced. This will help us save round trips between the developer and the CI process.

changed milestone to %Tor: 0.4.1.x-final

added 029-backport 035-backport 040-backport 041-backport 042-should actualpoints::.1 component::core tor/tor dgoulet-merge milestone::Tor: 0.4.1.x-final owner::nickm priority::medium resolution::fixed reviewer::teor severity::normal status::closed tor-ci type::enhancement labels

Travis isn't using "make -k" either, so maybe we should fix them both at the same time.

Sounds like a good idea to me.

Trac:
Description: Right now our .appveyor.yml is not using the "-k" make flag. That means that once it encounters an error, it stops building. In general, we would prefer our CI to keep trying to build for a long as it can, so that we can see all of the warnings and errors that are produced. This will help us save round trips between the developer and the CI process.

to

Right now our .appveyor.yml and .travis.yml are not using the "-k" make flag. That means that once it encounters an error, it stops building. In general, we would prefer our CI to keep trying to build for a long as it can, so that we can see all of the warnings and errors that are produced. This will help us save round trips between the developer and the CI process.
Summary: Appveyor should use "make -k" to Appveyor and Travis should use "make -k"

https://github.com/torproject/tor/pull/1228

Trac:
Status: new to needs_review

Looks good to me.

I think we should backport this change to 0.2.9 and later for consistency. Did you want to do the backport, rl1987? If not, I'm happy to do it when I review.

You might find the new "git-merge-forward.sh -t (test-branch-prefix)" in #31314 (moved) useful. (But this change will cause a few conflicts, unfortunately.)

Trac:
Type: defect to enhancement
Reviewer: N/A to teor
Status: needs_review to needs_revision
Keywords: N/A deleted, 041-backport, 029-backport, 040-backport, 035-backport added
Cc: N/A to nickm

Feel free to do the backporting.

Trac:
Cc: nickm to nickm, rl1987

Mark some needs_revision tickets as 042-should

Trac:
Keywords: N/A deleted, 042-should added

Changing owner.

Trac:
Owner: N/A to nickm
Status: needs_revision to accepted

Trac:
Status: accepted to needs_revision

I've made initial branches as ticket31372_029 for travis and ticket31372_appveyor_035 for appveyor.
For Travis, I just used the MAKEFLAGS variable, so that merging forward will be simpler.

For my own reference, the current PRs are: https://github.com/torproject/tor/pull/1350 for appveyor https://github.com/torproject/tor/pull/1351 for travis If CI passes, I'll make a combined branch for 0.3.5 and forward, and merge-forward branches for 0.4.0, 0.4.1, and 0.4.2.

CI passed.

ticket31372_029 (travis only) with PR: https://github.com/torproject/tor/pull/1351 ticket31372_035 (both platforms) with PR: https://github.com/torproject/tor/pull/1360 ticket31372_040 (small merge conflict) with PR: https://github.com/torproject/tor/pull/1361 ticket31372_041 (clean merge) with PR: https://github.com/torproject/tor/pull/1362 ticket31372_042 (clean merge) with PR: https://github.com/torproject/tor/pull/1363

I have also made a branch with a deliberate syntax error in mainloop.c so that we can make sure that make -k is working as in tended. That one has a PR at https://github.com/torproject/tor/pull/1364 -- do not merge it.

CI for the real branches has passed. For the intended-to-fail branch, I have confirmed that the build continues after the first failure, and generates more output (which is what we wanted make -k to do).

Trac:
Status: needs_revision to needs_review
Actualpoints: N/A to .1

I like this idea, but is there a way to summarise the errors at the end of the CI output? Otherwise, we have to search through thousands of lines of output for the error.

I do not know a way to summarize these errors offhand, without significant new coding.

We could run "make -k" twice, I guess? That way it will re-build whatever failed, thus outputting the errors again. That's a bit ugly though.

Usually I grep for "error:" or search for it in my browser.

I like make -k || make -k and I'm happy with the ugliness, because almost no-one will see it. (And we can explain it in a comment, and perhaps with echo "Re-running make to show errors".)

But I am also happy to change my process, and do a search.

What do you think would be easier for us, and for new contributors? I'll let you decide?

It's probably also worth mentioning that this change will slow down failures. I think that's ok, but I sent an email to the network team list with other ways to speed up CI.

I think that the search approach might be better: we have to do that anyway for jenkins builders, and it's a good idea to get in practice. Do you think that's plausible? If so, let's merge_ready this.

It's probably also worth mentioning that this change will slow down failures. I think that's ok, but I sent an email to the network team list with other ways to speed up CI.

Thanks! I also think it's okay.

The tradeoff is that we find out later when there is compilation failure, but we learn everything about the compilation failure and we don't have to do multiple back-and-forth steps. Learning about a success is just as slow as it was before.

I know that Appveyor will report failure as soon as one builder has failed. Does travis do the same?

Appveyor reports failure, and doesn't run any more jobs.

Travis runs all the jobs, then reports failures. It has a fast finish mode, which is buggy, and doesn't really do what we want anyway: https://docs.travis-ci.com/user/customizing-the-build#fast-finishing

Trac:
Status: needs_review to merge_ready

Trac:
Keywords: 042-should deleted, 042-should dgoulet-merge added

PR 1363 was merged!

Moving milestone to 041 for backport.

Trac:
Milestone: Tor: 0.4.2.x-final to Tor: 0.4.1.x-final

(Please feel free to backport this whenever it seems best to do so.)

Merged to 0.2.9 and later. Merged #32106 (moved), #31807 (moved), #31001 (moved), #23818 (moved), #12399 (moved), and #31372 (moved) together.

Trac:
Status: merge_ready to closed
Resolution: N/A to fixed

closed

added 48m of time spent

mentioned in issue #31807 (moved)

mentioned in issue #32106 (moved)

moved to tpo/core/tor#31372 (closed)