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

Fix for windows plug-in upgrades issue: #1623 #3401

Merged
merged 35 commits into from Sep 16, 2020

Conversation

BaronGreenback
Copy link
Contributor

@BaronGreenback BaronGreenback commented Jun 21, 2020

Plugins are stored in version marked folders. (eg. TMDb Box Set.4.0.0.0) with only the latest version installed at start up.

The attempted removal of each earlier versioned folders can be enabled to be done at startup. (Disabled in this change) If the folder is found to be locked, another attempt can be made at the next execution of Jellyfin. At worst, the plugins folder may have some older versions left behind - which will be ignored by the system.

The system is backwards compatible - only "version loading" the folder if a version number is included in the folder name.

#1623

@BaronGreenback
Copy link
Contributor Author

Is there a repository of older plugins which i could use for testing?

@crobibero
Copy link
Member

Is there a repository of older plugins which i could use for testing?

https://repo.jellyfin.org/releases/plugin/

@BaronGreenback BaronGreenback changed the title Possible solution to windows plug-in upgrades issue Fix for windows plug-in upgrades issue: #1623 Jun 21, 2020
@dkanada
Copy link
Member

dkanada commented Jun 21, 2020

@BaronGreenback a while ago Dolphin implemented a system to allow users to modify resource pack folders at will which I thought would be useful here. The plugin downloads would be inside a folder with a small file containing information like the GUID and version. This would allow any name for the files or folders as long as the metadata file is present, and would fall back to the existing behavior when missing. There would be some drawbacks to this method, but I figured I'd bring it up anyways for general discussion.

@BaronGreenback
Copy link
Contributor Author

BaronGreenback commented Jun 21, 2020

This versioning is basically to get around the issue of windows not being able to delete a locked file until reboot.

The package is already downloaded as a zip file - so any additional files could be included in that.
The JSON response from the plugin repository is the version number that routine uses for the folder creation - any internal versioning would have to match this one, or else run the risk of not being loaded.

What was the idea behind the metadata file? creating multi-plugin packages?

@BaronGreenback
Copy link
Contributor Author

Have noticed that notification pops up saying (object object), so i don't know if i;ve introduced a bug. eg OpenSubtitles version ([object] [object])

@mark-monteiro
Copy link
Member

I think you can get rid of a lot of the version parsing and comparison logic by using the Version class build into the framework: https://docs.microsoft.com/en-us/dotnet/api/system.version.parse?view=netcore-3.1#System_Version_Parse_System_String_

@EraYaN
Copy link
Member

EraYaN commented Jun 22, 2020

Nice, i like this a lot. We should probably store the manifest in the plugins zips (or generate it from the actual repo) and put it in manifest.json/yaml in the plugin folder. That way the name of the plugin folder is purely for informational purposes which seems more robust IMO, also lets you "disable" plugins by renaming the manifest or including a flag. And including the target server version too so a server upgrade can no longer break with a missing plugin or self heal by upgrading automatically.

@dkanada
Copy link
Member

dkanada commented Jun 22, 2020

If we store a manifest in the ZIP it would increase the burden on third party repositories though.

@EraYaN
Copy link
Member

EraYaN commented Jun 22, 2020

If we store a manifest in the ZIP it would increase the burden on third party repositories though.

That is fair, but Kodi repos can also deal with it perfectly. We might need some "build" tooling. It does make it very easy to make a repo manifest from a folder of zips though. (we could even just package the build.yaml with a different name)

@EraYaN
Copy link
Member

EraYaN commented Jul 31, 2020

The plugin zips now have the meta.json files, so this should now be a solvable issue. Without depending on the actual path name, but discovery can just rely on enumerating all meta.json files in the tree.

@BaronGreenback
Copy link
Contributor Author

@crobibero Named tuples implemented

Emby.Server.Implementations/ApplicationHost.cs Outdated Show resolved Hide resolved
Emby.Server.Implementations/ApplicationHost.cs Outdated Show resolved Hide resolved
Emby.Server.Implementations/ApplicationHost.cs Outdated Show resolved Hide resolved
Emby.Server.Implementations/Updates/InstallationManager.cs Outdated Show resolved Hide resolved
BaronGreenback and others added 3 commits September 8, 2020 17:49
Co-authored-by: Cody Robibero <cody@robibe.ro>
Co-authored-by: Cody Robibero <cody@robibe.ro>
Co-authored-by: Cody Robibero <cody@robibe.ro>
Co-authored-by: Cody Robibero <cody@robibe.ro>
BaronGreenback and others added 2 commits September 14, 2020 17:58
Co-authored-by: dkanada <dkanada@users.noreply.github.com>
BaronGreenback and others added 2 commits September 14, 2020 18:44
Co-authored-by: Cody Robibero <cody@robibe.ro>
Co-authored-by: Cody Robibero <cody@robibe.ro>
Co-authored-by: Cody Robibero <cody@robibe.ro>
Co-authored-by: h1dden-da3m0n <33120068+h1dden-da3m0n@users.noreply.github.com>
Co-authored-by: h1dden-da3m0n <33120068+h1dden-da3m0n@users.noreply.github.com>
Co-authored-by: h1dden-da3m0n <33120068+h1dden-da3m0n@users.noreply.github.com>
@EraYaN EraYaN added this to Active PRs in Release 10.7.0 via automation Sep 16, 2020
@EraYaN
Copy link
Member

EraYaN commented Sep 16, 2020

Alright this seems that it might be ready for a merge, correct?

@jellyfin/core

@BaronGreenback
Copy link
Contributor Author

yes - all the adaptations have been done that have been suggested.

@joshuaboniface
Copy link
Member

Great solution! If we weren't so diverged already I'd mark this a stable backport, but I feel like that would be hopeless. So 10.7.0 🚀

@joshuaboniface joshuaboniface merged commit bc5404c into jellyfin:master Sep 16, 2020
Release 10.7.0 automation moved this from Active PRs to Completed PRs Sep 16, 2020
@BaronGreenback BaronGreenback deleted the Plugins branch October 31, 2020 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Release 10.7.0
  
Completed PRs
Development

Successfully merging this pull request may close these issues.

None yet

7 participants