Re: [Ltib] Special tag localdir_nobuild causes LTIB to go to the config screen.

[aaron]

Hi Stuart.

Let's say I have a BSP with a kernel module that needs the kernel source
unpacked configured so that it can build an out-of-tree kernel module.

If I do a trelease/localdir_nobuild, then when the installer attempts to
follow the default instructions (cd non-*; ./ltib), the build should
succeed.  Eveything should build, the kernel, and the kernel module.  This
fix is already in place because we now avoid calling ./ltib -m clean for
localdir_nobuild.

If I do a trelease/localdir, then when the installer attempts to follow
the default instructions, the rootfs directory should be constructed from
the binary RPM files without doing any package builds.  This is the sort
of bug we have now, where an unnecessary kernel build is incurred.  In
this same case, if instead the installer types ./ltib --force in order to
force a rebuild of the target software, then the build should clearly
succeed, building the kernel and the kernel module.  This type of build
will currently not succeed, as *LEAVESRC will have been cleared in the
.config when constructing the iso, and the kernel module will not find the
kernel source, because it will have been cleared after building.

The patch I submitted takes care of these remaining cases, but might not
work in the case that you mentioned of selecting a new kernel module right
away.  With my patch, .config and .config.old with both have LEAVESRC=y,
but as you mention, since there is no change of state, and because change
of state is the method currently used to express this module/kernel
dependency, then this might not work.

Aside from this, I think the packager should have easy and explicit
control of the configuration presented to the installer of the BSP.  That
is to say, whatever defconfig file is being used should become the .config
that configures the release (and is used for the build of the release if a
build is performed), and this same .config should be in the iso BSP at the
time of a first installation.

In terms of speed, I think what Olivia was saying is that the kernel
doesn't need to have make called again in a kernel tree if it has already
been called in the case of an out-of-tree kernel module.  For an
out-of-tree kernel module, starting from scratch, you need the kernel
source unpacked, the .config copied, a make oldconfig, and a make modules.
 If these have been done once before, you don't need to have them done
again for selecting or de-selecting kernel module packages.  These
dependencies might be difficult to express, but there could be shortcuts
here.

Aaron

> Hi Aaron,
>
> I've been thinking about this a bit more.  My conclusion is that the
> problem:
>
> * occurs only when installing an ISO BSP
>
> * this is because it incorrectly makes decisions based on state data,
> which is clearly invalid on a first run/install.
>
> I'm thinking the solution is to detect a first install (or host package
> install) and not to process the state based triggers.
>
> I will look into implementing a change for this.  In the mean time can
> you let me know whether you think this would resolve the issues you have
> run into.
>
> Regards, Stuart
>
> address@hidden wrote:
>> Hi Stuart.
>>
>> In your test case below, another possible way to detect the need for a
>> kernel build is the absence of the kernel source code in the build
>> directory.  If PKG_KERNEL_LEAVESRC is set by something, and there is no
>> expanded kernel source, then you can cause a kernel build.  You wouldn't
>> need compare .config and .config.old in order to detect a state change
>> for
>> this case, so there is still not an explicit need for a transient flag.
>> A
>> caveat to this case is that the presence of the kernel source in the
>> build
>> directory does not necessarily mean that a kernel module will have the
>> complete infrastructure it needs to build.
>>
>> Aaron
>>
>>> Hi Aaron,
>>>
>>> As I explained before, this flag must be transient.  Take this use
>>> case:
>>>
>>> * user installed BSP with no builds occurring.
>>> * user selects a module that has not been previously built
>>>
>>> In this case you need the setting of LEAVESRC trigged by the module to
>>> be different from the previous state so you can trigger a kernel
>>> unpack/build.  If this flag were left already set (in the ISO preset)
>>> then you'd never get a change of state.
>>>
>>> The bug is as you point out that the kernel should not build on
>>> installation.  This needs investigation.
>>>
>>> Regards, Stuart
>>>
>>>
>>> address@hidden wrote:
>>>> Speaking honestly, for me the way this transient *LEAVESRC
>>>> configuration
>>>> variable is utilized right now is counter-intuitive, inflexible, and
>>>> prone
>>>> to unintended consequences.  In terms of being counter-intuitive, I
>>>> think
>>>> that the purpose of this variable is not clear because its transient
>>>> nature is obscured, and it cost me time to figure out what was going
>>>> on
>>>> and why I wasn't able to do what I thought I should be able to do
>>>> setting
>>>> up my BSP.  In terms of being inflexible, I had to try harder than
>>>> usual
>>>> to put in place what I wanted despite the functionality of *LEAVESRC.
>>>> In
>>>> terms of being prone to unintended consequences, it can lead to things
>>>> like unnecessary kernel builds, and many iterations of BSP generation
>>>> and
>>>> testing, which take a long time.
>>>>
>>>> I don't think there is any convenience in having this as a one-shot or
>>>> transient variable.  If the source of a package is expanded as a
>>>> result
>>>> of
>>>> setting this variable then I think both the source can stay expanded
>>>> and
>>>> the variable can stay set.  I think if its origin traces back to
>>>> trying
>>>> to
>>>> detect a state change in a dependency list, then it makes more sense,
>>>> but
>>>> I still feel it's wrong, because you don't need state information.  In
>>>> the
>>>> case of the kernel, if CONFIG_PKG_KERNEL_LEAVESRC=y, and there is no
>>>> directory rpm/BUILD/linux, then the kernel should be expanded with -m
>>>> prep, but not built purely as a result of *LEAVESRC, which might be
>>>> set
>>>> by
>>>> a kernel module that only needs the kernel's kbuild infrastructure.
>>>> If
>>>> the kernel package is already built in the BSP, then that's a
>>>> time-saver
>>>> for a developer who might only be working with a module, or a useful
>>>> logical partition for a developer who might want to prove that only
>>>> the
>>>> module is affected, and nothing in the kernel tree.  In fact, maybe it
>>>> should be renamed to EXPANDSRC, or something similar, and not treated
>>>> as
>>>> transient.  Any time this flag is set for any package, and the source
>>>> has
>>>> not been expanded in the build directory, a -m prep takes place, and
>>>> the
>>>> variable stays set.  If the user wants, the package source directory
>>>> can
>>>> be removed from the build directory, and the variable cleared in the
>>>> menu
>>>> configuration.
>>>>
>>>> I think in terms of the BSP feature, the more flexibility you can
>>>> offer
>>>> the better.  The user should be allowed some freedom to package their
>>>> board support software in the way that they want.  Even if they wanted
>>>> to
>>>> drop the user to a ltib/kernel/busybox configuration menu right away
>>>> (*WANT_CF), or force a complete target image rebuild
>>>> (localdir_nobuild),
>>>> then that should be possible.  This might seem goofy the way one
>>>> developer
>>>> thinks about it, but it might be useful to someone else.  Another
>>>> thing
>>>> I
>>>> would suggest is that if you're going to have a stage directory and
>>>> build
>>>> a BSP in that stage directory (which implicitly proves that the
>>>> software
>>>> of the BSP can be successfully built), then that same software and
>>>> configuration should be preserved in the BSP and presented to the BSP
>>>> installer, and not something slightly different.  Even if the chance
>>>> is
>>>> small, if something is slightly different the software might not
>>>> build.
>>>> I
>>>> ran into this.
>>>>
>>>> This is mostly interpretation stuff.  The only "bug" case at this
>>>> point
>>>> is:
>>>>
>>>> * Constructing a BSP with any tag other than localdir_nobuild that has
>>>> a
>>>> package that selects PKG_KERNEL_LEAVESRC.  The installer of the BSP
>>>> will
>>>> incur an unnecessary kernel build.
>>>>
>>>> Even this isn't really a bug, but things can work better, and they can
>>>> work worse.  Of course I'm not maintaining the LTIB, but that is from
>>>> my
>>>> perspective as a user.
>>>>
>>>> Aaron
>>>>
>>>>> Hi Aaron/Olivia,
>>>>>
>>>>> I've been thinking about this a little more (you have similar related
>>>>> issues I think).
>>>>>
>>>>> On common question that arises is whether or not the
>>>>> PKG_KERNEL_LEAVESRC
>>>>> should be transient.
>>>>>
>>>>> I believe it should because the principle is that when you install an
>>>>> ISO based LTIB image it should not build anything for the target
>>>>> image
>>>>> on the first ./ltib run, all it should do is re-populate the rootfs
>>>>> image based on the pre-built binary rpms.
>>>>>
>>>>> Given this, if you then for example enable a kernel module from the
>>>>> config system, this will set the transient flag: PKG_KERNEL_LEAVESRC
>>>>> This change of state will be picked up by LTIB and cause:
>>>>>
>>>>> * the kernel to unpack, build and keep the built source directory
>>>>> * the module itself to build
>>>>> * on exit the transient flag PKG_KERNEL_LEAVESRC
>>>>>
>>>>> On the other hand, if you were to not clear the  PKG_KERNEL_LEAVESRC
>>>>> in
>>>>> the .config for the ISO image, and you then enabled a module in the
>>>>> config system, this would set the already set PKG_KERNEL_LEAVESRC.
>>>>> As
>>>>> there is no change of state, ltib would not know to unpack the kernel
>>>>> sources.
>>>>>
>>>>> Notwithstanding this, there may be other bugs in this area, but to
>>>>> investigate I need a very simple (bullet points) test case I can
>>>>> understand.
>>>>>
>>>>> Regards, Stuart
>>>>>
>>>>> FYI, here's a run I did where I enabled the helloword module on a
>>>>> system
>>>>> that had already built the kernel but had no source code unpacked:
>>>>>
>>>>> Processing platform: A&MLtd Adder MPC875 PowerPC board
>>>>> ========================================================
>>>>> using config/platform/qs875s/.config
>>>>> PKG_KERNEL_LEAVESRC has changed, kernel-2.6.16-875 needs a rebuild
>>>>>
>>>>> Processing: fake-provides
>>>>> ===========================
>>>>>
>>>>> Processing: kernel-2.6.16-875
>>>>> ===============================
>>>>> Build path taken because: spec file newer than rpm,
>>>>>
>>>>>
>>>>>
>>>>> Stuart Hughes wrote:
>>>>>> Hi Aaron,
>>>>>>
>>>>>> Thanks, I'll take a look and think about this one and the
>>>>>> implications.
>>>>>>
>>>>>> Regards, Stuart
>>>>>>
>>>>>> address@hidden wrote:
>>>>>>> Hi Stuart.
>>>>>>>
>>>>>>> I think there's a comfort that the configuration used to create the
>>>>>>> BSP is
>>>>>>> the same configuration that is presented to the user when
>>>>>>> installing
>>>>>>> the
>>>>>>> BSP.  If the user wants to do a "./ltib --force" right away then it
>>>>>>> should
>>>>>>> be the same type of configuration and build that created the BSP.
>>>>>>> That's
>>>>>>> why I think the transient flags should not be affected in the
>>>>>>> release
>>>>>>> process.  I'm not a Perl programmer, so for sure this is not an
>>>>>>> elegant
>>>>>>> approach, but this patch is simple and works for me with both
>>>>>>> localdir
>>>>>>> and
>>>>>>> localdir_nobuild special tags.  Aside from preserving the
>>>>>>> configuration,
>>>>>>> it ensures that the kernel is not rebuilt in the case we discussed.
>>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Aaron
>>>>>>>
>>>>>>>> Hi Aaron,
>>>>>>>>
>>>>>>>> You have some points, however transient flags should not be relied
>>>>>>>> upon
>>>>>>>> for installs.  The most important IMHO you make is the unnecessary
>>>>>>>> kernel build on install, this is not meant to happen.  However
>>>>>>>> your
>>>>>>>> concern about rebuilding the kernel module should not be an issue
>>>>>>>> as
>>>>>>>> if
>>>>>>>> the module builds, it will should unpack and build the kernel.
>>>>>>>>
>>>>>>>> This a real corner case and I doubt I will have time to look into
>>>>>>>> it
>>>>>>>> in
>>>>>>>> the immediate future.  Could you try to come up with a patch that
>>>>>>>> implement what you need (with the minimum change of behaviour).
>>>>>>>>
>>>>>>>> Regards, Stuart
>>>>>>>>
>>>>>>>> address@hidden wrote:
>>>>>>>>> Hi Stuart.  I know this thread is from a while ago, and that this
>>>>>>>>> is
>>>>>>>>> probably a minor issue.  I just thought I'd mention that I think
>>>>>>>>> it
>>>>>>>>> might
>>>>>>>>> be more correct to avoid clearing these transient configuration
>>>>>>>>> flags
>>>>>>>>> (*WANT_CF and *LEAVESRC) when constructing a release or test
>>>>>>>>> release
>>>>>>>>> BSP.
>>>>>>>>>
>>>>>>>>> The thing is that even if I have a "select PKG_KERNEL_LEAVESRC"
>>>>>>>>> line
>>>>>>>>> somewhere, or otherwise have it configured, it will get cleared
>>>>>>>>> with
>>>>>>>>> the
>>>>>>>>> "./ltib -m clean" line in bin/Ltibrelease.pm when building a
>>>>>>>>> release.
>>>>>>>>> Now
>>>>>>>>> that the fix we worked on is present, the localdir_nobuild tag
>>>>>>>>> happily
>>>>>>>>> does not clear the transient flags automatically, but other tags
>>>>>>>>> such as
>>>>>>>>> localdir will.  This means that the configuration used to
>>>>>>>>> construct
>>>>>>>>> the
>>>>>>>>> BSP will differ from the configuration of the BSP when it's being
>>>>>>>>> installed from the iso (if any of the *WANT_CF or *LEAVESRC flags
>>>>>>>>> are
>>>>>>>>> set
>>>>>>>>> to 'y' in the default configuration) for all release tags except
>>>>>>>>> localdir_nobuild, and I think this is a little inconsistent.  I
>>>>>>>>> believe
>>>>>>>>> that the configuration that is present at the time the iso BSP is
>>>>>>>>> being
>>>>>>>>> built should be the equivalent configuration when the iso BSP is
>>>>>>>>> being
>>>>>>>>> installed.
>>>>>>>>>
>>>>>>>>> Particularly bad is the way the localdir special tag is working
>>>>>>>>> right
>>>>>>>>> now.
>>>>>>>>>  Since I have a kernel module that depends on the kernel sources
>>>>>>>>> being
>>>>>>>>> unpacked, I have my select PKG_KERNEL_LEAVESRC line in the
>>>>>>>>> package.lkc.
>>>>>>>>> I
>>>>>>>>> go to make a test release, and this builds the kernel binary RPM
>>>>>>>>> as
>>>>>>>>> expected, but after it's built, the LTIB script clears
>>>>>>>>> CONFIG_PKG_KERNEL_LEAVESRC in the .config file that is
>>>>>>>>> distributed
>>>>>>>>> with
>>>>>>>>> the BSP.  When the user goes to install the BSP from the iso with
>>>>>>>>> "cd
>>>>>>>>> non-*; ./ltib", the kernel package is actually automatically
>>>>>>>>> rebuilt
>>>>>>>>> because there is a file .config.old that has
>>>>>>>>> CONFIG_PKG_KERNEL_LEAVESRC=y,
>>>>>>>>> and this differs with the .config file.  So, in effect, it
>>>>>>>>> rebuilds
>>>>>>>>> the
>>>>>>>>> kernel binary RPM unnecessarily, but on top of that, it clears
>>>>>>>>> the
>>>>>>>>> kernel
>>>>>>>>> source when it's finished, because in the new .config file
>>>>>>>>> CONFIG_PKG_KERNEL_LEAVESRC is not set.  Supposing a user wants to
>>>>>>>>> force
>>>>>>>>> a
>>>>>>>>> rebuild of the kernel module at this point, the kernel source
>>>>>>>>> will
>>>>>>>>> not
>>>>>>>>> be
>>>>>>>>> available in the BUILD directory.
>>>>>>>>>
>>>>>>>>> Perhaps it might work for only the "./ltib -m clean" operation to
>>>>>>>>> avoid
>>>>>>>>> clearing transient flags, and then these flags will persist
>>>>>>>>> through
>>>>>>>>> this
>>>>>>>>> call, and be present in the same state in the BSP as they were to
>>>>>>>>> build
>>>>>>>>> the BSP.
>>>>>>>>>
>>>>>>>>> What do you think?
>>>>>>>>>
>>>>>>>>> Aaron
>>>>>>>>>
>>>>>>>>>> Hi Aaron,
>>>>>>>>>>
>>>>>>>>>> PKG_KERNEL_LEAVESRC is meant to be a one-shot as once unpacked
>>>>>>>>>> the
>>>>>>>>>> source will remain and so there's no need to keep the flag
>>>>>>>>>> enabled.
>>>>>>>>>>
>>>>>>>>>> If you package needs the kernel source unpacked it should be
>>>>>>>>>> selecting
>>>>>>>>>> PKG_KERNEL_LEAVESRC rather than relying on manually setting this
>>>>>>>>>> in
>>>>>>>>>> the
>>>>>>>>>> ./ltib -m config session.  Take a look at
>>>>>>>>>> config/userspace/package.lkc,
>>>>>>>>>> for example:
>>>>>>>>>>
>>>>>>>>>> config PKG_HELLOWORLD_MOD
>>>>>>>>>>      depends CAP_HAS_MMU
>>>>>>>>>>      select PKG_KERNEL_LEAVESRC
>>>>>>>>>>      bool "hello world module example"
>>>>>>>>>>      help
>>>>>>>>>>          simple hello world kernel modules example
>>>>>>>>>>
>>>>>>>>>> Regards, Stuart
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> address@hidden wrote:
>>>>>>>>>>> Hi Peter and Stuart.  Working with my kernel module is
>>>>>>>>>>> precisely
>>>>>>>>>>> where
>>>>>>>>>>> this gave me a little trouble.  I understand that you need the
>>>>>>>>>>> kernel
>>>>>>>>>>> source unpacked for a kernel module package.  My question was
>>>>>>>>>>> why
>>>>>>>>>>> this
>>>>>>>>>>> is
>>>>>>>>>>> a one-shot or transient variable.  The area that it was giving
>>>>>>>>>>> me
>>>>>>>>>>> a
>>>>>>>>>>> few
>>>>>>>>>>> problems was after creating an iso release, the build and
>>>>>>>>>>> installation
>>>>>>>>>>> of
>>>>>>>>>>> the BSP onto another machine.  Using `./ltib` I would attempt
>>>>>>>>>>> to
>>>>>>>>>>> build
>>>>>>>>>>> the
>>>>>>>>>>> whole project (host and target packages), and I would get stuck
>>>>>>>>>>> on
>>>>>>>>>>> the
>>>>>>>>>>> kernel module package because my kernel source would not remain
>>>>>>>>>>> in
>>>>>>>>>>> the
>>>>>>>>>>> BUILD directory.  This was because the PKG_KERNEL_LEAVESRC=y
>>>>>>>>>>> that
>>>>>>>>>>> I
>>>>>>>>>>> had
>>>>>>>>>>> put in place earlier was getting cleared automatically by a
>>>>>>>>>>> call
>>>>>>>>>>> to
>>>>>>>>>>> `./ltib` when building the iso.  Since I was running into this
>>>>>>>>>>> sort of
>>>>>>>>>>> problem, I was wondering if there is any beneficial reason for
>>>>>>>>>>> having
>>>>>>>>>>> the
>>>>>>>>>>> LEAVESRC variable as a one-shot or transient variable.  It
>>>>>>>>>>> seems
>>>>>>>>>>> like
>>>>>>>>>>> if
>>>>>>>>>>> you have the source out for a particular package, and then you
>>>>>>>>>>> wanted
>>>>>>>>>>> to
>>>>>>>>>>> get rid of it, you can just remove it, and change the
>>>>>>>>>>> configuration to
>>>>>>>>>>> comment out any variables like PKG_KERNEL_LEAVESRC if
>>>>>>>>>>> necessary.
>>>>>>>>>>> Updating
>>>>>>>>>>> the date/time stamp on my config/platform/${board}/.config file
>>>>>>>>>>> does
>>>>>>>>>>> not
>>>>>>>>>>> force a rebuild of the kernel for me.
>>>>>>>>>>>
>>>>>>>>>>>> One to thik about is a seperate package that builds a kernel
>>>>>>>>>>>> module
>>>>>>>>>>>> (like PKG_HELLO_WORLD_MOD).  It needs the kernel source to
>>>>>>>>>>>> remain
>>>>>>>>>>>> so
>>>>>>>>>>>> it
>>>>>>>>>>>> can use the kernel build structure/headers to create a module
>>>>>>>>>>>> that is
>>>>>>>>>>>> loadable by the kernel....
>>>>>>>>>>>> Hi Aaron,
>>>>>>>>>>>>
>>>>>>>>>>>> Once source code is blown out and left (with LEAVESRC) it will
>>>>>>>>>>>> stay
>>>>>>>>>>>> there. nothing will remove it until you do manually, so the
>>>>>>>>>>>> flag
>>>>>>>>>>>> only
>>>>>>>>>>>> needs to be transient.  The need for this is as Peter says,
>>>>>>>>>>>> some
>>>>>>>>>>>> packages (like kernel modules) need access to the the real
>>>>>>>>>>>> built
>>>>>>>>>>>> kernel
>>>>>>>>>>>> sources.
>>>>>>>>>>>>
>>>>>>>>>>>> --leavesrc --force should leave all the sources unpacked.
>>>>>>>>>>>> This
>>>>>>>>>>>> is
>>>>>>>>>>>> generally speaking a bad idea.
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks for the patch.
>>>>>>>>>>>>
>>>>>>>>>>>> Regards, Stuart
>>>>>>>>>>> _______________________________________________
>>>>>>>>>>> LTIB home page: http://ltib.org
>>>>>>>>>>>
>>>>>>>>>>> Ltib mailing list
>>>>>>>>>>> address@hidden
>>>>>>>>>>> http://lists.nongnu.org/mailman/listinfo/ltib
>>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> _______________________________________________
>>>>>>>>> LTIB home page: http://ltib.org
>>>>>>>>>
>>>>>>>>> Ltib mailing list
>>>>>>>>> address@hidden
>>>>>>>>> http://lists.nongnu.org/mailman/listinfo/ltib
>>>>>>>>>
>>>>>>>> ------------------------------------------------------------------------
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> LTIB home page: http://ltib.org
>>>>>>>>
>>>>>>>> Ltib mailing list
>>>>>>>> address@hidden
>>>>>>>> http://lists.nongnu.org/mailman/listinfo/ltib
>>>>>> _______________________________________________
>>>>>> LTIB home page: http://ltib.org
>>>>>>
>>>>>> Ltib mailing list
>>>>>> address@hidden
>>>>>> http://lists.nongnu.org/mailman/listinfo/ltib
>>>>>>
>>>>
>>>>
>>
>>
>>
>