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.

Sign up for GitHub

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

Repo update usability fixes #3249

Merged
merged 1 commit into from
Jan 9, 2021
Merged

Repo update usability fixes #3249

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 0 additions & 2 deletions Cmdline/Action/Update.cs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
8 changes: 6 additions & 2 deletions GUI/Main/Main.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,8 +359,12 @@ 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);
transaction.Complete();
}
}

base.OnFormClosing(e);
Expand Down
6 changes: 6 additions & 0 deletions GUI/Main/MainInstall.cs
View file
Open in desktop
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