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

Shared folder strategies #39

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .idea/.idea.StabilityMatrix/.idea/.gitignore

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions .idea/.idea.StabilityMatrix/.idea/encodings.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions .idea/.idea.StabilityMatrix/.idea/indexLayout.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions .idea/.idea.StabilityMatrix/.idea/vcs.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion StabilityMatrix.Tests/Helper/PackageFactoryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public void Setup()
fakeBasePackages = new List<BasePackage>
{
// TODO: inject mocks
new DankDiffusion(null, null, null, null)
new DankDiffusion(null, null, null, null, null)
};
packageFactory = new PackageFactory(fakeBasePackages);
}
Expand Down
4 changes: 3 additions & 1 deletion StabilityMatrix/App.xaml.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.IO;
Expand Down Expand Up @@ -231,6 +231,8 @@ private void App_OnStartup(object sender, StartupEventArgs e)
serviceCollection.AddSingleton<BasePackage, A3WebUI>();
serviceCollection.AddSingleton<BasePackage, VladAutomatic>();
serviceCollection.AddSingleton<BasePackage, ComfyUI>();
serviceCollection.AddSingleton<VladAutomaticSharedFolderStrategy>();
serviceCollection.AddTransient<LinkedFolderSharedFolderStrategy>();
serviceCollection.AddSingleton<Wpf.Ui.Contracts.ISnackbarService, Wpf.Ui.Services.SnackbarService>();
serviceCollection.AddSingleton<IPrerequisiteHelper, PrerequisiteHelper>();
serviceCollection.AddSingleton<ISnackbarService, SnackbarService>();
Expand Down
10 changes: 8 additions & 2 deletions StabilityMatrix/Models/Packages/A3WebUI.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,14 @@ public class A3WebUI : BaseGitPackage
public string RelativeArgsDefinitionScriptPath => "modules.cmd_args";


public A3WebUI(IGithubApiCache githubApi, ISettingsManager settingsManager, IDownloadService downloadService,
IPrerequisiteHelper prerequisiteHelper) :
public A3WebUI(IGithubApiCache githubApi,
ISettingsManager settingsManager,
IDownloadService downloadService,
IPrerequisiteHelper prerequisiteHelper,
LinkedFolderSharedFolderStrategy sharedFolderStrategy) :
base(githubApi, settingsManager, downloadService, prerequisiteHelper)
{
SharedFolderStrategy = sharedFolderStrategy;
}

// From https://github.com/AUTOMATIC1111/stable-diffusion-webui/tree/master/models
Expand All @@ -48,6 +52,8 @@ public A3WebUI(IGithubApiCache githubApi, ISettingsManager settingsManager, IDow
[SharedFolderType.ControlNet] = "models/ControlNet"
};

public override ISharedFolderStrategy SharedFolderStrategy { get; protected set; }

[SuppressMessage("ReSharper", "ArrangeObjectCreationWhenTypeNotEvident")]
public override List<LaunchOptionDefinition> LaunchOptions => new()
{
Expand Down
6 changes: 5 additions & 1 deletion StabilityMatrix/Models/Packages/BaseGitPackage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,11 @@ public abstract class BaseGitPackage : BasePackage
protected readonly IDownloadService DownloadService;
protected readonly IPrerequisiteHelper PrerequisiteHelper;
protected PyVenvRunner? VenvRunner;


public BaseGitPackage()
{

}
/// <summary>
/// URL of the hosted web page on launch
/// </summary>
Expand Down
8 changes: 5 additions & 3 deletions StabilityMatrix/Models/Packages/BasePackage.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
Expand Down Expand Up @@ -31,13 +31,15 @@ public abstract class BasePackage

public abstract List<LaunchOptionDefinition> LaunchOptions { get; }
public virtual string? ExtraLaunchArguments { get; set; } = null;

/// <summary>
/// The shared folders that this package supports.
/// Mapping of <see cref="SharedFolderType"/> to the relative path from the package root.
/// </summary>
public virtual Dictionary<SharedFolderType, string>? SharedFolders { get; }


public abstract ISharedFolderStrategy SharedFolderStrategy { get; protected set; }
Copy link
Contributor

@mohnjiles mohnjiles Jul 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it may be better to implement an abstract method, along the lines of ExecuteSharedFolderStrategy instead - we could inject the ISharedFolders to the implementations of this, so that the packages without custom strategies can just call the original SharedFolders method without the is LinkedFolderSharedFolderStrategy check.

You could still inject the strategies to the classes that need them, we just wouldn't expose the whole strategy object to the callers. You might also need to pass in the directory as a parameter, since the BasePackage doesn't know where it's installed (most of the time).

It might also eliminate the need for LinkedFolderSharedFolderStrategy and avoid the circular reference shenanigans.

I hope that made sense 😅


public abstract Task<string> GetLatestVersion();
public abstract Task<IEnumerable<PackageVersion>> GetAllVersions(bool isReleaseMode = true);
public abstract Task<IReadOnlyList<GitHubCommit>?> GetAllCommits(string branch, int page = 1, int perPage = 10);
Expand Down
7 changes: 5 additions & 2 deletions StabilityMatrix/Models/Packages/ComfyUI.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,10 @@ public class ComfyUI : BaseGitPackage
public override bool ShouldIgnoreReleases => true;

public ComfyUI(IGithubApiCache githubApi, ISettingsManager settingsManager, IDownloadService downloadService,
IPrerequisiteHelper prerequisiteHelper) :
IPrerequisiteHelper prerequisiteHelper, LinkedFolderSharedFolderStrategy sharedFolderStrategy) :
base(githubApi, settingsManager, downloadService, prerequisiteHelper)
{
SharedFolderStrategy = sharedFolderStrategy;
}

// https://github.com/comfyanonymous/ComfyUI/blob/master/folder_paths.py#L11
Expand All @@ -46,7 +47,9 @@ public ComfyUI(IGithubApiCache githubApi, ISettingsManager settingsManager, IDow
[SharedFolderType.ESRGAN] = "models/upscale_models",
[SharedFolderType.Hypernetwork] = "models/hypernetworks",
};


public override ISharedFolderStrategy SharedFolderStrategy { get; protected set; }

public override List<LaunchOptionDefinition> LaunchOptions => new()
{
new()
Expand Down
5 changes: 4 additions & 1 deletion StabilityMatrix/Models/Packages/DankDiffusion.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ namespace StabilityMatrix.Models.Packages;
public class DankDiffusion : BaseGitPackage
{
public DankDiffusion(IGithubApiCache githubApi, ISettingsManager settingsManager, IDownloadService downloadService,
IPrerequisiteHelper prerequisiteHelper) :
IPrerequisiteHelper prerequisiteHelper, LinkedFolderSharedFolderStrategy sharedFolderStrategy) :
base(githubApi, settingsManager, downloadService, prerequisiteHelper)
{
SharedFolderStrategy = sharedFolderStrategy;
}

public override string Name => "dank-diffusion";
Expand All @@ -27,6 +28,8 @@ public override Task RunPackage(string installedPackagePath, string arguments)
}

public override List<LaunchOptionDefinition> LaunchOptions { get; }
public override ISharedFolderStrategy SharedFolderStrategy { get; protected set; }

public override Task<string> GetLatestVersion()
{
throw new System.NotImplementedException();
Expand Down
8 changes: 8 additions & 0 deletions StabilityMatrix/Models/Packages/ISharedFolderStrategy.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
using System.Threading.Tasks;

namespace StabilityMatrix.Models.Packages;

public interface ISharedFolderStrategy
{
Task ExecuteAsync(BasePackage package);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
using System;
using System.Threading.Tasks;
using Microsoft.Extensions.DependencyInjection;

namespace StabilityMatrix.Models.Packages;

public class LinkedFolderSharedFolderStrategy : ISharedFolderStrategy
{
private readonly IServiceProvider serviceProvider;

public LinkedFolderSharedFolderStrategy(IServiceProvider serviceProvider)
{
this.serviceProvider = serviceProvider;
}

public Task ExecuteAsync(BasePackage package)
{
// TODO: Move SharedFolders logic here
// NOTE: We're using this awkward solution because a circular dependency is generated in the graph otherwise
var sharedFolders = serviceProvider.GetRequiredService<ISharedFolders>();
sharedFolders.SetupLinksForPackage(package, package.InstallLocation);
return Task.CompletedTask;
}
}
5 changes: 4 additions & 1 deletion StabilityMatrix/Models/Packages/VladAutomatic.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,10 @@ public class VladAutomatic : BaseGitPackage
public override bool ShouldIgnoreReleases => true;

public VladAutomatic(IGithubApiCache githubApi, ISettingsManager settingsManager, IDownloadService downloadService,
IPrerequisiteHelper prerequisiteHelper) :
IPrerequisiteHelper prerequisiteHelper, VladAutomaticSharedFolderStrategy sharedFolderStrategy) :
base(githubApi, settingsManager, downloadService, prerequisiteHelper)
{
SharedFolderStrategy = sharedFolderStrategy;
}

// https://github.com/vladmandic/automatic/blob/master/modules/shared.py#L324
Expand All @@ -53,6 +54,8 @@ public VladAutomatic(IGithubApiCache githubApi, ISettingsManager settingsManager
[SharedFolderType.LyCORIS] = "models/LyCORIS",
};

public override ISharedFolderStrategy SharedFolderStrategy { get; protected set; }

[SuppressMessage("ReSharper", "ArrangeObjectCreationWhenTypeNotEvident")]
public override List<LaunchOptionDefinition> LaunchOptions => new()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Threading.Tasks;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using StabilityMatrix.Extensions;
using StabilityMatrix.Helper;
using StabilityMatrix.Models.FileInterfaces;

namespace StabilityMatrix.Models.Packages;

public class VladAutomaticSharedFolderStrategy : ISharedFolderStrategy
{
private readonly ISettingsManager settingsManager;

public VladAutomaticSharedFolderStrategy(ISettingsManager settingsManager)
{
this.settingsManager = settingsManager;
}

public async Task ExecuteAsync(BasePackage package)
{
var installedPackage = settingsManager
.Settings
.InstalledPackages
.Single(p => p.PackageName == package.Name);
Comment on lines +22 to +27
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few things here -

  1. In the InstallerViewModel, this is called before the InstalledPackage object has been added to the settings, so this will throw an exception unless they had a previous install of the same package (in which case it'd be using the wrong paths anyway).

  2. It is possible to have two different installs of the same package, so using .Single() with the PackageName here could possibly throw. If possible, its best to use the Guid Id when trying to find an InstalledPackage.

  3. Since the name property is all that's used from the BasePackage, could the method parameter here just be the name of the package instead of the full thing?

var configFilePath = Path.Combine(settingsManager.LibraryDir, installedPackage.LibraryPath!, "config.json");

// Load the configuration file
var json = await File.ReadAllTextAsync(configFilePath);
var job = JsonConvert.DeserializeObject<JObject>(json)!;
Comment on lines +31 to +32
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've been using System.Text.Json for our json parsing, as it has improved quite a bit since the initial release. So that we don't have two different json parsing libraries involved, would it be possible to redo this using System.Text.Json? I believe it should have a similar class to JObject called JsonObject


// Update the configuration values
var modelsDirectory = new DirectoryPath(settingsManager.ModelsDirectory);
foreach (var (sharedFolderType, configKey) in map)
{
var value = Path.Combine(modelsDirectory.FullPath, sharedFolderType.GetStringValue());
job[configKey] = value;
}

// Write the configuration file
await File.WriteAllTextAsync(configFilePath, JsonConvert.SerializeObject(job, Formatting.Indented));
}

private Dictionary<SharedFolderType, string> map = new()
{
{ SharedFolderType.StableDiffusion, "ckpt_dir" },
{ SharedFolderType.Diffusers, "diffusers_dir" },
{ SharedFolderType.VAE, "vae_dir" },
{ SharedFolderType.Lora, "lora_dir" },
{ SharedFolderType.LyCORIS, "lyco_dir" },
// { SharedFolderType.Styles, "styles_dir"},
{ SharedFolderType.TextualInversion, "embeddings_dir" },
{ SharedFolderType.Hypernetwork, "hypernetwork_dir" },
{ SharedFolderType.Codeformer, "codeformer_models_path" },
{ SharedFolderType.GFPGAN, "gfpgan_models_path" },
{ SharedFolderType.ESRGAN, "esrgan_models_path" },
{ SharedFolderType.BSRGAN , "bsrgan_models_path"},
{ SharedFolderType.RealESRGAN, "realesrgan_models_path" },
{ SharedFolderType.ScuNET, "scunet_models_path" },
{ SharedFolderType.SwinIR, "swinir_models_path" },
{ SharedFolderType.LDSR, "ldsr_models_path" },
{ SharedFolderType.CLIP, "clip_models_path" }
};
}

1 change: 1 addition & 0 deletions StabilityMatrix/StabilityMatrix.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
<PackageReference Include="Microsoft.Toolkit.Uwp.Notifications" Version="7.1.3" />
<PackageReference Include="Microsoft.Web.WebView2" Version="1.0.1823.32" />
<PackageReference Include="Microsoft.Xaml.Behaviors.Wpf" Version="1.1.39" />
<PackageReference Include="Newtonsoft.Json" Version="13.0.1" />
<PackageReference Include="NLog" Version="5.2.0" />
<PackageReference Include="NLog.Extensions.Logging" Version="5.3.0" />
<PackageReference Include="NSec.Cryptography" Version="22.4.0" />
Expand Down
5 changes: 3 additions & 2 deletions StabilityMatrix/ViewModels/InstallerViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -318,8 +318,9 @@ private async Task ActuallyInstall()
await InstallPackage();

ProgressText = "Setting up shared folder links...";
sharedFolders.SetupLinksForPackage(SelectedPackage, SelectedPackage.InstallLocation);


await SelectedPackage.SharedFolderStrategy.ExecuteAsync(SelectedPackage);

ProgressText = "Done";
IsIndeterminate = false;
ProgressValue = 100;
Expand Down
5 changes: 4 additions & 1 deletion StabilityMatrix/ViewModels/LaunchViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,10 @@ private void ToastNotificationManagerCompatOnOnActivated(
basePackage.StartupComplete += RunningPackageOnStartupComplete;

// Update shared folder links (in case library paths changed)
sharedFolders.UpdateLinksForPackage(basePackage, packagePath);
if (basePackage.SharedFolderStrategy is LinkedFolderSharedFolderStrategy)
sharedFolders.UpdateLinksForPackage(basePackage, packagePath);
else
await basePackage.SharedFolderStrategy.ExecuteAsync(basePackage);
Comment on lines +160 to +163
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: could you please put curly braces around these statements? According to our style guidelines:

Only omit curly braces from if statements if the statement immediately following is a return.

Thank you!


// Load user launch args from settings and convert to string
var userArgs = settingsManager.GetLaunchArgs(activeInstall.Id);
Expand Down