Skip to content

Commit

Permalink
Repo update usability fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
@HebaruSan
HebaruSan committed Jan 8, 2021
1 parent ad66e3c commit c988406
Show file tree
Hide file tree
Showing 8 changed files with 107 additions and 67 deletions.
2 changes: 0 additions & 2 deletions Cmdline/Action/Update.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@ public int RunCommand(CKAN.KSP ksp, object raw_options)

List<CkanModule> compatible_prior = null;

user.RaiseMessage("Downloading updates...");

if (options.list_changes)
{
// Get a list of compatible modules prior to the update.
Expand Down
21 changes: 20 additions & 1 deletion Cmdline/ConsoleUser.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ public int RaiseSelectionDialog(string message, params object[] args)
/// <param name="args">Possible arguments to format the message</param>
public void RaiseError(string message, params object[] args)
{
GoToStartOfLine();
if (Headless)
{
// Special GitHub Action formatting for mutli-line errors
Expand All @@ -260,6 +261,7 @@ public void RaiseError(string message, params object[] args)
{
Console.Error.WriteLine(message, args);
}
atStartOfLine = true;
}

/// <summary>
Expand Down Expand Up @@ -287,8 +289,11 @@ public void RaiseProgress(string message, int percent)
// The percent looks weird on non-download messages.
// The leading newline makes sure we don't end up with a mess from previous
// download messages.
Console.Write("\r\n{0}", message);
GoToStartOfLine();
Console.Write("{0}", message);
}
// These messages leave the cursor at the end of a line of text
atStartOfLine = false;
}

/// <summary>
Expand All @@ -303,7 +308,21 @@ public void RaiseProgress(string message, int percent)
/// <param name="args">Arguments to format the message</param>
public void RaiseMessage(string message, params object[] args)
{
GoToStartOfLine();
Console.WriteLine(message, args);
atStartOfLine = true;
}

private void GoToStartOfLine()
{
if (!atStartOfLine)
{
// Carriage return
Console.WriteLine("");
atStartOfLine = true;
}
}

private bool atStartOfLine = true;
}
}
11 changes: 4 additions & 7 deletions ConsoleUI/ModListScreen.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -416,11 +416,8 @@ private int daysSinceUpdated(string filename)

private bool UpdateRegistry()
{
ConsoleMessageDialog d = new ConsoleMessageDialog(
"Updating registry...",
new List<string>()
);
d.Run(() => {
ProgressScreen ps = new ProgressScreen("Updating Registry", "Checking for updates");
LaunchSubScreen(ps, () => {
HashSet<string> availBefore = new HashSet<string>(
Array.ConvertAll<CkanModule, string>(
registry.CompatibleModules(
Expand All @@ -435,11 +432,11 @@ private bool UpdateRegistry()
RegistryManager.Instance(manager.CurrentInstance),
manager.CurrentInstance,
manager.Cache,
this
ps
);
} catch (Exception ex) {
// There can be errors while you re-install mods with changed metadata
RaiseError(ex.Message + ex.StackTrace);
ps.RaiseError(ex.Message + ex.StackTrace);
}
// Update recent with mods that were updated in this pass
foreach (CkanModule mod in registry.CompatibleModules(
Expand Down
26 changes: 19 additions & 7 deletions Core/Net/Repo.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,19 @@ public static class Repo
public static RepoUpdateResult UpdateAllRepositories(RegistryManager registry_manager, KSP ksp, NetModuleCache cache, IUser user)
{
SortedDictionary<string, Repository> sortedRepositories = registry_manager.registry.Repositories;
user.RaiseProgress("Checking for updates", 0);
if (sortedRepositories.Values.All(repo => !string.IsNullOrEmpty(repo.last_server_etag) && repo.last_server_etag == Net.CurrentETag(repo.uri)))
{
user?.RaiseMessage("No changes since last update");
user.RaiseProgress("Already up to date", 100);
user.RaiseMessage("No changes since last update");
return RepoUpdateResult.NoChanges;
}
List<CkanModule> allAvail = new List<CkanModule>();
int index = 0;
foreach (KeyValuePair<string, Repository> repository in sortedRepositories)
{
log.InfoFormat("About to update {0}", repository.Value.name);
user.RaiseProgress($"Updating {repository.Value.name}",
10 + 80 * index / sortedRepositories.Count);
SortedDictionary<string, int> downloadCounts;
string newETag;
List<CkanModule> avail = UpdateRegistry(repository.Value.uri, ksp, user, out downloadCounts, out newETag);
Expand All @@ -58,18 +62,24 @@ public static RepoUpdateResult UpdateAllRepositories(RegistryManager registry_ma
}
else
{
log.InfoFormat("Updated {0}", repository.Value.name);
// Merge all the lists
allAvail.AddRange(avail);
repository.Value.last_server_etag = newETag;
user.RaiseMessage("Updated {0}", repository.Value.name);
}
++index;
}
// Save allAvail to the registry if we found anything
if (allAvail.Count > 0)
{
registry_manager.registry.SetAllAvailable(allAvail);
// Save our changes.
registry_manager.Save(enforce_consistency: false);
user.RaiseProgress("Saving modules to registry", 90);
using (var transaction = CkanTransaction.CreateTransactionScope())
{
// Save our changes.
registry_manager.registry.SetAllAvailable(allAvail);
registry_manager.Save(enforce_consistency: false);
transaction.Complete();
}

ShowUserInconsistencies(registry_manager.registry, user);

Expand All @@ -81,6 +91,8 @@ public static RepoUpdateResult UpdateAllRepositories(RegistryManager registry_ma

// Registry.CompatibleModules is slow, just return success,
// caller can check it if it's really needed
user.RaiseProgress("Registry saved", 100);
user.RaiseMessage("Repositories updated");
return RepoUpdateResult.Updated;
}
else
Expand Down Expand Up @@ -110,7 +122,7 @@ private static List<CkanModule> UpdateRegistry(Uri repo, KSP ksp, IUser user, ou
}
catch (System.Net.WebException ex)
{
user.RaiseMessage("Failed to download {0}: {1}", repo, ex.ToString());
user.RaiseError("Failed to download {0}: {1}", repo, ex.Message);
currentETag = null;
return null;
}
Expand Down
99 changes: 51 additions & 48 deletions Core/Registry/Registry.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,6 @@ public void SetDownloadCounts(SortedDictionary<string, int> counts)
private Dictionary<string, HashSet<AvailableModule>> providers
= new Dictionary<string, HashSet<AvailableModule>>();

[JsonIgnore] private string transaction_backup;

/// <summary>
/// Returns all the activated registries, sorted by priority and name
/// </summary>
Expand Down Expand Up @@ -317,9 +315,15 @@ public static Registry Empty()

#region Transaction Handling

// We use this to record which transaction we're in.
// Which transaction we're in
private string enlisted_tx;

// JSON serialization of self when enlisted with tx
private string transaction_backup;

// Coordinate access of multiple threads to the tx info
private readonly object txMutex = new object();

// This *doesn't* get called when we get enlisted in a Tx, it gets
// called when we're about to commit a transaction. We can *probably*
// get away with calling .Done() here and skipping the commit phase,
Expand All @@ -344,16 +348,13 @@ public void Commit(Enlistment enlistment)
// Hooray! All Tx participants have signalled they're ready.
// So we're done, and can clear our resources.

enlisted_tx = null;
transaction_backup = null;
log.DebugFormat("Committing registry tx {0}", enlisted_tx);
lock (txMutex) {
enlisted_tx = null;
transaction_backup = null;

enlistment.Done();
log.Debug("Registry transaction committed");

// TODO: Should we save to disk at the end of a Tx?
// TODO: If so, we should abort if we find a save that's while a Tx is in progress?
//
// In either case, do we want the registry_manager to be Tx aware?
enlistment.Done();
}
}

public void Rollback(Enlistment enlistment)
Expand All @@ -363,14 +364,16 @@ public void Rollback(Enlistment enlistment)
// In theory, this should put everything back the way it was, overwriting whatever
// we had previously.

var options = new JsonSerializerSettings {ObjectCreationHandling = ObjectCreationHandling.Replace};
lock (txMutex) {
var options = new JsonSerializerSettings {ObjectCreationHandling = ObjectCreationHandling.Replace};

JsonConvert.PopulateObject(transaction_backup, this, options);
JsonConvert.PopulateObject(transaction_backup, this, options);

enlisted_tx = null;
transaction_backup = null;
enlisted_tx = null;
transaction_backup = null;

enlistment.Done();
enlistment.Done();
}
}

private void SaveState()
Expand All @@ -382,41 +385,45 @@ private void SaveState()
}

/// <summary>
/// "Pardon me, but I couldn't help but overhear you're in a Transaction..."
///
/// Adds our registry to the current transaction. This should be called whenever we
/// do anything which may dirty the registry.
/// </summary>
//
// http://wondermark.com/1k62/
private void SealionTransaction()
private void EnlistWithTransaction()
{
// This property is thread static, so other threads can't mess with our value
if (Transaction.Current != null)
{
string current_tx = Transaction.Current.TransactionInformation.LocalIdentifier;

if (enlisted_tx == null)
// Multiple threads might be accessing this shared state, make sure they play nice
lock (txMutex)
{
log.Debug("Pardon me, but I couldn't help overhear you're in a transaction...");
Transaction.Current.EnlistVolatile(this, EnlistmentOptions.None);
SaveState();
enlisted_tx = current_tx;
}
else if (enlisted_tx != current_tx)
{
throw new TransactionalKraken("CKAN registry does not support nested transactions.");
}
if (enlisted_tx == null)
{
log.DebugFormat("Enlisting registry with tx {0}", current_tx);
// Let's save our state before we enlist and potentially allow ourselves
// to be reverted by outside code
SaveState();
Transaction.Current.EnlistVolatile(this, EnlistmentOptions.None);
enlisted_tx = current_tx;
}
else if (enlisted_tx != current_tx)
{
throw new TransactionalKraken(
$"Registry already enlisted with tx {enlisted_tx}, can't enlist with tx {current_tx}");
}

// If we're here, it's a transaction we're already participating in,
// so do nothing.
// If we're here, it's a transaction we're already participating in,
// so do nothing.
}
}
}

#endregion

public void SetAllAvailable(IEnumerable<CkanModule> newAvail)
{
SealionTransaction();
EnlistWithTransaction();
// Clear current modules
available_modules = new Dictionary<string, AvailableModule>();
providers.Clear();
Expand All @@ -443,7 +450,7 @@ public bool HasAnyAvailable()
/// </summary>
public void AddAvailable(CkanModule module)
{
SealionTransaction();
EnlistWithTransaction();

var identifier = module.identifier;
// If we've never seen this module before, create an entry for it.
Expand Down Expand Up @@ -471,7 +478,7 @@ public void RemoveAvailable(string identifier, ModuleVersion version)
AvailableModule availableModule;
if (available_modules.TryGetValue(identifier, out availableModule))
{
SealionTransaction();
EnlistWithTransaction();
availableModule.Remove(version);
}
}
Expand Down Expand Up @@ -708,7 +715,7 @@ public CkanModule GetModuleByVersion(string ident, ModuleVersion version)
/// </summary>
public void RegisterModule(CkanModule mod, IEnumerable<string> absolute_files, KSP ksp, bool autoInstalled)
{
SealionTransaction();
EnlistWithTransaction();

sorter = null;

Expand Down Expand Up @@ -770,7 +777,7 @@ public void RegisterModule(CkanModule mod, IEnumerable<string> absolute_files, K
/// </summary>
public void DeregisterModule(KSP ksp, string module)
{
SealionTransaction();
EnlistWithTransaction();

sorter = null;

Expand Down Expand Up @@ -812,7 +819,7 @@ public void DeregisterModule(KSP ksp, string module)
/// </summary>
public void RegisterDll(KSP ksp, string absolute_path)
{
SealionTransaction();
EnlistWithTransaction();

string relative_path = ksp.ToRelativeGameDir(absolute_path);

Expand Down Expand Up @@ -858,7 +865,7 @@ public void RegisterDll(KSP ksp, string absolute_path)
/// </summary>
public void ClearDlls()
{
SealionTransaction();
EnlistWithTransaction();
installed_dlls = new Dictionary<string, string>();
}

Expand Down Expand Up @@ -942,13 +949,9 @@ public Dictionary<string, ModuleVersion> Installed(bool withProvides = true, boo
/// </summary>
public InstalledModule InstalledModule(string module)
{
// In theory, someone could then modify the data they get back from
// this, so we sea-lion just in case.

SealionTransaction();

InstalledModule installedModule;
return installed_modules.TryGetValue(module, out installedModule) ? installedModule : null;
return installed_modules.TryGetValue(module, out InstalledModule installedModule)
? installedModule
: null;
}

/// <summary>
Expand Down
7 changes: 5 additions & 2 deletions GUI/Main/Main.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -359,8 +359,11 @@ protected override void OnFormClosing(FormClosingEventArgs e)

if (needRegistrySave)
{
// Save registry
RegistryManager.Instance(CurrentInstance).Save(false);
using (var transaction = CkanTransaction.CreateTransactionScope())
{
// Save registry
RegistryManager.Instance(CurrentInstance).Save(false);
}
}

base.OnFormClosing(e);
Expand Down
6 changes: 6 additions & 0 deletions GUI/Main/MainInstall.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,12 @@ private void PostInstallMods(object sender, RunWorkerCompletedEventArgs e)
currentUser.RaiseError(dlcMsg);
break;

case TransactionalKraken exc:
// Want to see the stack trace for this one
currentUser.RaiseMessage(exc.ToString());
currentUser.RaiseError(exc.ToString());
break;

default:
currentUser.RaiseMessage(e.Error.Message);
break;
Expand Down
Loading

0 comments on commit c988406

Please sign in to comment.