Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AddonInstaller] - 1 year and 8 month after the last recursion fix fr… #10444

Merged
merged 1 commit into from Sep 13, 2016

Conversation

@Memphiz
Copy link
Member

commented Sep 9, 2016

…om me - lets fix the recursion again.

Well my last try worked by accident i think:

02d50b1

Not sure what the circular dependency was back then - but it was different.

Finally i think it makes most sense to add the current addon id whose dependencies are checked to the predeps list before checking its dependencies. Because this finally leads to the fact that we don't check an addon and its depends twice.

@akva2 and @tamland for a second thought.

Reproduction of the crash as follows:

  1. remove any trace of script.module.t9.search, script.extendedinfo and script.module.kodi65 from addons folder
  2. download and install http://mirrors.kodi.tv/addons/krypton/script.module.t9.search/script.module.t9.search-1.1.0.zip
  3. This will fetch script.kodi65 1.1.0 from the repo as a dependency which is already a fixed version without circular dependency.
  4. Downgrade that by downloading and installing http://mirrors.kodi.tv/addons/krypton/script.module.kodi65/script.module.kodi65-1.0.3.zip which is the version which has the circular dependency in it
  5. After that - install script.extendedinfo from our repo
  6. crash and burn in recursion

Apply this PR and see the crash goes away.

@MilhouseVH can you give this a try in libreelec please?

@MilhouseVH

This comment has been minimized.

Copy link
Contributor

commented Sep 9, 2016

Yep, will include it in tonight's build - thanks.

@Memphiz

This comment has been minimized.

Copy link
Member Author

commented Sep 13, 2016

Jenkins build this please - @tamland @notspiff - no comment is a good comment? ;)

@notspiff

This comment has been minimized.

Copy link
Contributor

commented Sep 13, 2016

Well it is absolutely effing clear that tamland is absolute and doesnt care
what others say so my only comment is 'ha ha'.

  1. sep. 2016 07:21 skrev "Memphiz" notifications@github.com:

Jenkins build this please - @tamland https://github.com/tamland
@notspiff https://github.com/notspiff - no comment is a good comment? ;)


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#10444 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AFktYKuCXqGEoF5baGF6Xffa1pY91Jv9ks5qpjLzgaJpZM4J5UeT
.

@Memphiz

This comment has been minimized.

Copy link
Member Author

commented Sep 13, 2016

Haha on the bugfix? No clue what i got into here...

@notspiff

This comment has been minimized.

Copy link
Contributor

commented Sep 13, 2016

Not for the bug fix.

  1. sep. 2016 12:27 skrev "Memphiz" notifications@github.com:

Haha on the bugfix? No clue what i got into here...


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#10444 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AFktYP5KQ_jDqtMobWt4mScyDSRDep17ks5qpnp5gaJpZM4J5UeT
.

@tamland

This comment has been minimized.

Copy link
Member

commented Sep 13, 2016

Me neither. I've never touched this code before..

@tamland tamland merged commit abf1aa5 into xbmc:master Sep 13, 2016

1 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
jenkins.kodi.tv You are a failure. Fix the code and try again......
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.