On Wed, Dec 6, 2017 at 8:02 PM, Alfredo Deza <adeza@xxxxxxxxxx> wrote: > On Wed, Dec 6, 2017 at 2:44 AM, kefu chai <tchaikov@xxxxxxxxx> wrote: >> On Wed, Dec 6, 2017 at 4:27 AM, Alfredo Deza <adeza@xxxxxxxxxx> wrote: >>> On Tue, Dec 5, 2017 at 12:44 PM, Gregory Farnum <gfarnum@xxxxxxxxxx> wrote: >>>> On Tue, Dec 5, 2017 at 5:31 AM, Alfredo Deza <adeza@xxxxxxxxxx> wrote: >>>>> It seems like we keep hitting catastrophic build errors with minor doc >>>>> changes. The last one was introduced by a change I made to the >>>>> ceph-disk man page that updated the title: >>>>> >>>>> https://github.com/ceph/ceph/pull/19241/commits/bd00560c20caeeb1ded211cb81533280338014d1 >>>>> >>>>> That change builds correctly (!) and will render just fine, and it was >>>>> merged after the docs build reported as successful. >>>>> >>>>> However, this also made all subsequent binary builds break. This >>>>> disconnect has caused similar issues in the past. At some point the >>>>> docs build was completely broken for days, because there was no >>>>> visibility from pull requests. >>>>> >>>>> To mitigate that, we now have the docs job building for every pull >>>>> request, regardless if it is only editing rst files (because, yes, one >>>>> can break the docs build with C++). >>>>> >>>>> This has helped a lot, but it still falls short because the binary >>>>> build process for Ceph is different from how we build docs (including >> >> please note, the cmake building system builds all the artifacts we ship to >> our enduser. in addition to the binaries, it also builds and intalls scripts, >> documents, sample configurations, copyright, and more. it's more than >> merely a "binary build process". i'd call it a "build process". =) >> >>>>> man pages). This is why a successful docs build in a PR can still >>>>> break a binary build. >>>> >>>> Do we understand why it failed on the binary build but not the docs build? >>>> >>>> Seems like that would help us see if we can replicate any other issues. >>> >>> It failed there because the man pages are generated differently, using >>> a couple of ad-hoc helpers that try to ensure certain conditions. In >>> this case, it was a Python >>> utility (ceph/man/conf.py). It was trying to make sure that the title >>> of the man page matched the name of the man page. >>> I had added "[DEPRECATED]" to the ceph-disk title, which failed the assertion. >>> >>> But because the utilities use plain asserts, you don't get any of that >>> rich error reporting, just that some value != some other value. >>> >>> Regardless, the reason is because the code paths are not the same, so >>> we need to find a way to either make them the same, or try to report >>> whatever code paths >>> we need to touch for a PR. >>> >>>> >>>>> There are a couple of things that could help: >>>>> >>>>> 1) I don't entirely follow how CMake is building the man/doc pages, >>>>> but a job that tries to replicate that path could catch these >>>>> inconsistencies in PRs. >> >> could you define "a job"? we have a target for building the man pages. >> "make manpages" will build all of them. > > See, I didn't know :) > > However! That would only build the man pages. I've broken builds > before when a man page that was expected > by ceph.spec wasn't there. `make manpages` would get us closer though, > this is great. By 'job' I meant A jenkins job that could check this. > >> >>>>> >>>>> 2) Build binaries on every PR, and report them as status. >> >> yeah, i guess that's what "needs-qa" is for. > > That lacks visibility on reporting back to the PR, and it is a very > manual process that someone has to go through. My concern is to solve > this > in a programmatic way that has a way to notify the author and reviewers of a PR. i see. but probably you can use more reviewers next time. this is how i do with my PRs: check the recent changes of the files the PR is changing, and find the authors and reviewers of them, then add them as reviewer or @ them . well, i know, it's still not a programmatic way. =( > > I realize that building binaries for every PR is just not practical > for us at the pace they get produced. >> >>>>> >>>>> I don't know enough of CMake to attempt #1, and #2 would cause >>>>> contributors a lot of pain because it would involve having to push to >>>>> ceph-ci.git always. >>>>> >>>>> I am open to any other ideas or suggestions here. >>> -- >>> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in >>> the body of a message to majordomo@xxxxxxxxxxxxxxx >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> >> >> -- >> Regards >> Kefu Chai -- Regards Kefu Chai -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
