Re: [PATCH RFC] insane.bbclass: add buildpaths_cmake and buildpaths_pkgconfig checks

[<Mikko.Rapeli@bmw.de>]

On Tue, Sep 25, 2018 at 07:29:22PM +0100, richard.purdie@linuxfoundation.org wrote:
> On Tue, 2018-09-25 at 17:11 +0000, Mikko.Rapeli@bmw.de wrote:
> > On Tue, Sep 25, 2018 at 04:19:39PM +0100, richard.purdie@linuxfoundat
> > ion.org wrote:
> > > On Tue, 2018-09-25 at 11:21 +0000, Mikko.Rapeli@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.

Ok, I'll see what I can do about the tests...

> > 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...

I wonder what would be exposed if recipe builds would really be confined
in their sysroots with seccomp or filesystem namespaces..

-Mikko