[OE-core] [PATCH RFC] insane.bbclass: add buildpaths_cmake and buildpaths_pkgconfig checks

[richard.purdie at linuxfoundation.org]

On Tue, 2018-09-25 at 17:11 +0000, Mikko.Rapeli at bmw.de wrote:
> On Tue, Sep 25, 2018 at 04:19:39PM +0100, richard.purdie at linuxfoundat
> ion.org wrote:
> > On Tue, 2018-09-25 at 11:21 +0000, Mikko.Rapeli at bmw.de wrote:
> > > On Tue, Sep 25, 2018 at 12:08:24PM +0100, Richard Purdie wrote:
> > > > I suspect we need to talk to cmake upstream about fixing this
> > > > properly
> > > > but adding something in the class may be an option until a
> > > > better
> > > > upstream solution can be found.
> > > > 
> > > > I am puzzled by the need to add a .pc file path check since I
> > > > thought
> > > > there was already a test for that in insane.bbclass?
> > > > 
> > > > package_qa_check_staged maybe? "Check staged la and pc files
> > > > for
> > > > common
> > > > problems like references to the work directory."
> > > 
> > > That check is not enabled by default. At least bash is producing
> > > a
> > > broken
> > > bash.pc (and several other files like Makefile.in and bashbug) in
> > > sumo
> > > with embedded absolute paths to build sysroot.
> > 
> > I still felt I was missing something:
> > 
> > ERROR_QA ?= "dev-so debug-deps dev-deps debug-files arch pkgconfig
> > la 
> > 
> > which sets "pkgconfig" and "la". package_qa_check_staged calls
> > package_qa_handle_error("la",...) and
> > package_qa_handle_error("pkgconfig"...).
> > 
> > do_qa_staging calls package_qa_check_staged() and is triggered by:
> > 
> > do_populate_sysroot[postfuncs] += "do_qa_staging "
> > 
> > which is set for everything and should be running on master?
> > 
> > I had a look at bash.pc in a random local build and there is no
> > hardcoded path in it for master. I then found a sumo build and
> > looked
> > at bash.pc there and also no hardcoded paths.
> > 
> > The issues would have appeared to have been fixed by:
> > 
> > http://git.yoctoproject.org/cgit.cgi/poky/commit/?id=dabbfe23de0615
> > a958fac31b83684ade024cf17d 
> > which should be in sumo.
> 
> Cool, but this ignores nativesdk, which is where I saw the problems
> with this test. Somehow missed that nativesdk part in my reply,
> possibly because of plain recipe name in the error message.

With the addition of nativesdk, this starts to make more sense...

> > What may be the real issue is that sanity checker is quite specific
> > about what its looking for and does do:
> > 
> > file_content = file_content.replace(recipesysroot, "")
> > 
> > which may make sense for .la files but perhaps not .pc files, I'd
> > guess
> > its to stop compiler flags triggering errors. 
> > 
> > So the real fix here may be to remove that line from the .pc check,
> > at
> > least in the target case (native case it would make sense)?
> 
> And .cmake files?

We should add something to cover those. I was just worried about why
tests I thought were there weren't working. I do think we should
fix/improve the existing pkgconfig test rather than add another similar
one.

> I've fixed issues found by this test in my tree by following the
> pattern from these "improve reproducibility" fixes. Some recipes were
> installing generated .cmake files from build tree (bad, bad) and
> several recipes were somehow generating .cmake files with absolute
> paths for libraries. It's completely unclear to me when and why CMake
> decides to fill in absolute paths, like with libical.

I'm afraid you probably know more about cmake than I do at this
point...

Cheers,

Richard