[rainman74]

@garbear that would be great if you could do that!

[garbear]

The problem is more difficult than I thought. I'm a week in and feel like I've barely made progress. I'll keep chugging away.

[rainman74]

The problem is more difficult than I thought. I'm a week in and feel like I've barely made progress. I'll keep chugging away.

 Are you the only Kodi developer around, or can you consult someone else? Please hold on!

[garbear]

Back in march I wrote a HOW-TO on killing globals so we could refactor the login screen, and already four other devs have stepped up:
 
 

• Window manager global: https://github.com/xbmc/xbmc/pull/13722 (fernet)
  
• Info manager global: https://github.com/xbmc/xbmc/pull/13776 (ksooo)
  
• Network services global: https://github.com/xbmc/xbmc/pull/13785 (afedchin)
  
• Audio manager global: https://github.com/xbmc/xbmc/pull/14198 (hudokkow)
  
• Color manager global: https://github.com/xbmc/xbmc/pull/14199 (hudokkow)
  

The problem now is our circular dependencies. Binary add-ons depend on profiles, but profiles depend on binary add-ons. I'm writing a HOW-TO on killing circular dependencies so that we can get more devs working on this task.

Our circular dependency problem is much worse than I thought. We have literally hundreds of thousands of lines of code that depend on profiles, whereas profiles currently depend on 23 separate components totaling many tens of thousands of lines. It could easily be another 10-20 hours before I break the first circular dependency and get that tutorial written. Just to let you know, there's a high chance I won't be able to do this by release next month.

[Koying]

@garbear Aren't you looking too hard for the specific issue of the profile selection screen?

I don't see it having any functional service dependencies, really, it's basically a selection dialog box from the list of profiles.
So it might be easier to refactor that one rather than trying to break every single circular dependencies...

[garbear]

The login screen isn't the problem (despite my explanations). The problem is the act of logging in/out, whether a login screen is shown or not. This uses profiles. Here's what profiles depends on: https://github.com/garbear/xbmc/blob/285...pp#L35-L59

Profiles controls services directly, but everything depends on profiles. Services need to register themselves with profiles and start/stop/restart when the appropriate callbacks (which don't exist yet) are fired. We need to remove the dependence on those 23 headers I linked above. The act of doing so, by putting each service in control of how it response to profile events, will solve the problem.

[Koying]

Mmm... Let's rewind a bit.

The specific problem at hand started here: https://github.com/xbmc/xbmc/commit/7e98...21d5bR1144
where you now unconditionally execute InitStageThree(), whether the profile login screen is to be shown or not.

Is it mandatory for some reason that this init happens there rather than when the final profile is loaded?

[scott967]

I can't say I understand anything of the design, but I guess a certain amount of the masterprofile context has to be loaded, then the login screen window runs in the masterprofile context.  A couple problems I have had include
1.  When loading the masterprofile context, the masterprofile skin's startup.xml gets run.  If the startup runs scripts from window <onload> statements, that can cause issues.
2.  After selecting the new profile, when the new profile's skin loads startup.xml doesn't run for some reason, but jumps right into home window.  To get my skin to work, I had to create some home window properties to see if my scripts from startup.xml were run or not.
3.  I have a specific instance where instead of showing the login screen, Kodi actually loads the masterprofile and I end up in the masterproffile skin's home window.  I haven't been able to recreate that in a clean test environment to narrow down what is going on.

scott s.
.

[garbear]

Mmm... Let's rewind a bit.

The specific problem at hand started here: https://github.com/xbmc/xbmc/commit/7e98...21d5bR1144
where you now unconditionally execute InitStageThree(), whether the profile login screen is to be shown or not.

Is it mandatory for some reason that this init happens there rather than when the final profile is loaded?

  
Yes: https://github.com/xbmc/xbmc/pull/13412

I can't say I understand anything of the design

That's the problem - there is no design. For a decade and a half devs have been adding features and hacking in profile support in the most direct, dirty way. My goal is to implement an actual design so that things load, unload and reload when they are supposed to. This has already become a team-wide effort spanning months, and the difficult part (breaking circular dependencies) is just beginning.

You've intuited well that the login screen runs in the master profile context. Unfortunately stuff bleeds into other profiles and the GUI. I'm very interested in seeing if you can find any other problems with the current system. Maybe starting with a small, feasible task will help me tackle the larger problem.

[Koying]

Mmm... Let's rewind a bit.

The specific problem at hand started here: https://github.com/xbmc/xbmc/commit/7e98...21d5bR1144
where you now unconditionally execute InitStageThree(), whether the profile login screen is to be shown or not.

Is it mandatory for some reason that this init happens there rather than when the final profile is loaded?

  
Yes: https://github.com/xbmc/xbmc/pull/13412  

Oh, so you need it there for inputmanager?
Probably afedchin's solution is better, then, and setting back InitStageThree back where it was...

[garbear]

You mean never call stage three if the login screen is enabled? Now we have to add hacks for every service in stage three that is skipped. Anyways, I asked him to update and merge his PR... but it was already fixed :p

[Koying]

You mean never call stage three if the login screen is enabled?

 Haha, no, to do it in https://github.com/xbmc/xbmc/blob/master...r.cpp#L268 , somehow.
Not sure why that one doesn't use stages, probably just not refactored, yet. At some point, I guess the function should become a bunch of deinit's and init's after switching profile...

[garbear]

Haha, no, to do it in https://github.com/xbmc/xbmc/blob/master...r.cpp#L268 , somehow.

The problem is that devs do exactly this, and have forever. Do you see how this creates a circular dependency?

The profiles system is the very most root dependency in Kodi: Settings are profile-dependent, and everything uses settings. Passwords are profile-dependent, and every file and directory goes through passwords. The list goes on.

Anything that profiles pulls into its source is something that it depends on. Therefore, every system that profiles directly accesses is circular. I currently count 23 such dependencies: https://github.com/garbear/xbmc/blob/285...pp#L35-L59 . Ouch.

With such circular dependencies, it is virtually impossible to get the order of init/deinit/reinit right. This would require a deep analysis of every path between every system in the entire codebase, and no one can hold that dependency graph in their head.

So.

My goal is to break these circular dependencies. To see what that process looks like (from PR14304):

Peripherals use InputManager all over the place. It registers itself for callbacks and injects input. However, a circular dependency exists - InputManager "cheats" by using peripherals through CServiceBroker: https://github.com/xbmc/xbmc/pull/14304/...b3445cL102 . This is a circular dependency.

Breaking a circular dependency is straight-forward. First, I identified the base functionality that InputManager needed from Peripherals. In this case, it was getting a single keypress. So I extracted this functionality into an interface: https://github.com/xbmc/xbmc/blob/b55a89...ntSource.h

Next, I make Peripherals implement this interface: https://github.com/xbmc/xbmc/pull/14304/...8399de4R60

Then, Peripherals registers itself with InputManager *behind* the interface: https://github.com/xbmc/xbmc/pull/14304/...653df3dR89

Now, InputManager can access this functionality through the interface: https://github.com/xbmc/xbmc/pull/14304/...b3445cR105 - notice how it no longer uses Peripherals directly. Circular dependency broken!

[garbear]

Do you still feel doing it in profiles is the correct path?

[rainman74]

@garbear Until a solution is found, you couldn't just setting back InitStageThree back where it was?