Bug 321883

Summary: duplicate kde-projects modules improperly handled with option handling
Product: [Developer tools] kdesrc-build Reporter: Michael Pyne <mpyne>
Component: project metadataAssignee: Michael Pyne <mpyne>
Status: RESOLVED FIXED    
Severity: normal CC: faure
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: Other   
OS: Linux   
Latest Commit: Version Fixed In: 1.16

Description Michael Pyne 2013-07-03 02:57:07 UTC
It is easiest to describe with the testcase (developed when trying to add an option to filter out phases in the config file):
=====
module-set
    repository kde-projects
    # Manually specify juk so we can override options
    use-modules kde/kdemultimedia juk
end module-set

module juk
    filter-out-phases update
end module
=====

The idea is that "module juk" locates the existing ksb::Module object that would be have created for the module-set just before it, and edits the options of that object directly. This actually *does* work, too, at this stage of the code there is:

[ksb::Module("kde/kdemultimedia"), ksb::Module("juk*")] # "juk*" is the modified ksb::Module

Later in the code kdesrc-build performs its kde-projects expansion pass, which converts the module list to

[ksb::Module("ffmpegthumbs"), ..., ksb::Module("juk"), ..., ksb::Module("juk*")]

i.e. there are now 2 juk entries, which is not supposed to be possible.

It turns out that they appear to be filtered out again by the dependency resolver; but it only picks the first one it sees, which in this case is the unmodified "juk" variant. This causes the options specified for the module to not work, as they get filtered right back out again by the code.

Workaround: Specify most-specific modules earlier in the use-modules so that the kde-projects expansion ends up created the dup'd ksb::Module to be filtered out.

However this should be fixed for real, probably with some kind of ksb::Module registry to be maintained by the build context and passed around.
Comment 1 David Faure 2013-11-19 08:55:28 UTC
I think this is the same issue:


module-set
  repository kde-projects
  use-modules yakuake calligra libktorrent ktorrent konversation libmm-qt libnm-qt networkmanagement k3b kregexpeditor nepomukshell polkit-kde-agent-1 skanlite libkscreen kscreen
  branch master
end module-set

module libmm-qt
  # Opensuse 12.3 has ModemManager 0.6
  branch mm_0_6_branch
end module

Shouldn't this use branch mm_0_6_branch for libmm-qt? It uses master instead.
Comment 2 Michael Pyne 2013-11-20 23:36:06 UTC
Yes, that should be correct syntax. I think I was working on a fix for this before I ran out of time, let me see if I can go and finally get this fixed for good.
Comment 3 Michael Pyne 2013-12-03 02:07:00 UTC
Getting close...

kde-svn@midna ~/projects/meta-kdesrc-build-tests/test-bug-321883 $ kdesrc-build -p libmm-qt
Script started processing at Mon Dec  2 21:05:58 2013
 * Using existing projects.kde.org project database, output may change
 * when database is updated next.

Building libmm-qt from <module-set at line 10> (1/1)
        Cloning libmm-qt
        Would have downloaded snapshot for libmm-qt
        http://anongit.kde.org/libmm-qt/libmm-qt-latest.tar.gz
        Switching to branch mm_0_6_branch
        Would have run 'git' 'checkout' '-b' 'mm_0_6_branch' 'origin/mm_0_6_branch'
        Source update complete for libmm-qt: 1 file affected.
        Preparing build system for libmm-qt.
        Would have cleaned build system for libmm-qt
        Would have created /home/kde-svn/kdesrc/build/extragear/libs/libmm-qt
        Running cmake...
        Compiling...
        Build succeeded after 0 seconds.
        Installing libmm-qt
        Would have installed libmm-qt
        Overall time for libmm-qt was 0 seconds.

<<<  PACKAGES SUCCESSFULLY BUILT  >>>
libmm-qt
Comment 4 Michael Pyne 2013-12-03 03:25:25 UTC
Git commit 319eb52b9b4604b4f4a9a43aa8202b85eb644d37 by Michael Pyne.
Committed on 29/11/2013 at 21:51.
Pushed by mpyne into branch 'option-handling-fixes'.

Fix bugs in option application to module-set modules.

Handling options for the modules generated from a module-set has always been
especially clunky, and at some point even the hacks failed and it became
impossible to do things like setting an option for a specific module picked out
of a larger module-set.

E.g. the idea was always that the following would work:

    module-set foo
        repository kde-projects
        use-modules kdelibs kde-workspace kde-runtime
        cmake-options -DCMAKE_BUILD_TYPE=Release
    end module-set

    # Override the above, but only for kde-runtime
    module kde-runtime
        cmake-options -DCMAKE_BUILD_TYPE=Debug
    end module

Probably I should have named "module" as "options" or even "override" (and
that's still feasible), but in any event this has been broken for awhile:
whenever kde-runtime is built it would end up with a Release build type instead
of a Debug one.

Worse yet, the second mention of kde-runtime was handled as a separate module.
Usually this duplicate module was weeded out by accident during the dependency
resolution phase, but that leaves open the question of which internal Module
object was the "winner"... now we can't even rely on the option handling being
predictably broken.

This was partially helped by c565d4c which at least prevents the code from
spitting out duplicate modules from within a given module-set (which is quite
easy to do by accident with kdelibs).

This commit reorganizes the command line and option reading code to do the
following:

- Add a "pending option" tracker, to hold option values that should be applied
  to a named module, if one happens to be created, either via explicit
  module-set expansion (e.g. if you ask for nepomuk-core or nepomuk-widgets) or
  via implicit expansion (e.g. if you ask for kdemultimedia, you get juk and a
  lot of others).
- Add a "selector" method which is responsible for translating module entries
  on the command line into appropriate module-set or module selections from the
  Modules and ModuleSets read in from the rc-file.
- Pass a subroutine to the module-set expansion method and the selector method
  to ensure that any new Modules created as a part of either process are checked
  for pending options (either from the rc-file due to the "override" module
  method, or from the command line.
- While I was digging it out I made the long-overdue switch to Getopt::Long
  away from my custom option parser. There are a couple of minor features lost in
  this process but if they are needed I can add them back.

Now it should be possible to override options for individual modules within a
module-set. However as before, the module you wish to override *must* have been
mentioned somewhere before in a use-modules entry so that kdesrc-build can
recognize that it must hold onto those option values for later application. It
is still safe to mention a kde-projects module and its superset, they will be
re-ordered if necessary.
FIXED-IN:1.16

M  +586  -559  modules/ksb/Application.pm
M  +9    -0    modules/ksb/ModuleSet.pm

http://commits.kde.org/kdesrc-build/319eb52b9b4604b4f4a9a43aa8202b85eb644d37
Comment 5 Michael Pyne 2013-12-03 03:27:13 UTC
Please note the different branch if you would like to test to see that the invasive changes don't seem to break anything (at least until I can catch the test suite up with the move to ksb::Application). If it seems to work for you I (or you) can merge to master, it should be a linear rebase at this point.
Comment 6 David Faure 2013-12-17 21:30:43 UTC
Works for my libmm-qt example.

To ensure there are no regressions elsewhere I'd have to run a full build at the office tomorrow.