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

Aborting a ckan scan halfway through leaves a corrupt registry.json file #2754

Closed
lamont-granquist opened this issue May 7, 2019 · 1 comment · Fixed by #2755
Closed
Assignees
Labels
Core (ckan.dll) Issues affecting the core part of CKAN Registry Issues affecting the registry

Comments

@lamont-granquist
Copy link

Think this is related to or possibly a spiritual-duplicate of #1114

I don't really want to recreate the issue at this point and I've since repaired my installation, but the use case was simple, which was to do ckan scan on the commandline and then ctrl-c it halfway through it. I actually had typed something like ckan scan Proc when I meant ckan search Proc so I just quickly ctrl-c'd it to fix the typo.

What resulted was a registry.json which was clearly half-written and truncated, and any subsequent ckan command was throwing JSON parsing errors. Doing a ckan repair registry didn't help any.

It's probably a simple case of where the routines that write out JSON files need to write to a tempfile first, then close the filehandle, then move/rename the file into place.

@DasSkelett DasSkelett added Bug Something is not working as intended Registry Issues affecting the registry labels May 7, 2019
@HebaruSan HebaruSan added the Core (ckan.dll) Issues affecting the core part of CKAN label May 7, 2019
@HebaruSan
Copy link
Member

HebaruSan commented May 7, 2019

We've already got a file system transaction library, maybe we can use that?

EDIT: We already do!

public void Save(bool enforce_consistency = true)
{
TxFileManager file_transaction = new TxFileManager();
log.InfoFormat("Saving CKAN registry at {0}", path);
if (enforce_consistency)
{
// No saving the registry unless it's in a sane state.
registry.CheckSanity();
}
string directoryPath = Path.GetDirectoryName(path);
if (directoryPath == null)
{
log.ErrorFormat("Failed to save registry, invalid path: {0}", path);
throw new DirectoryNotFoundKraken(path, "Can't find a directory in " + path);
}
if (!Directory.Exists(directoryPath))
{
Directory.CreateDirectory(directoryPath);
}
file_transaction.WriteAllText(path, Serialize());
ExportInstalled(
Path.Combine(directoryPath, $"installed-{ksp.Name}.ckan"),
false, true
);
if (!Directory.Exists(ksp.InstallHistoryDir()))
{
Directory.CreateDirectory(ksp.InstallHistoryDir());
}
ExportInstalled(
Path.Combine(
ksp.InstallHistoryDir(),
$"installed-{ksp.Name}-{DateTime.Now.ToString("yyyy-MM-dd_HH-mm-ss")}.ckan"
),
false, true
);
}

That raises some questions. Does the transaction not work? Are we using it wrong?

EDIT II: The scan operation has a transaction, but the Save call takes place outside of (after) it. That looks like a bad idea.

CKAN/Core/KSP.cs

Lines 409 to 445 in f2209e2

public void ScanGameData()
{
var manager = RegistryManager.Instance(this);
using (TransactionScope tx = CkanTransaction.CreateTransactionScope())
{
manager.registry.ClearDlls();
// TODO: It would be great to optimise this to skip .git directories and the like.
// Yes, I keep my GameData in git.
// Alas, EnumerateFiles is *case-sensitive* in its pattern, which causes
// DLL files to be missed under Linux; we have to pick .dll, .DLL, or scanning
// GameData *twice*.
//
// The least evil is to walk it once, and filter it ourselves.
IEnumerable<string> files = Directory.EnumerateFiles(
GameData(),
"*",
SearchOption.AllDirectories
);
files = files.Where(file => Regex.IsMatch(file, @"\.dll$", RegexOptions.IgnoreCase));
foreach (string dll in files.Select(KSPPathUtils.NormalizePath))
{
var relativePath = ToRelativeGameDir(dll);
if (!DllIgnoreList.Contains(relativePath))
manager.registry.RegisterDll(this, dll);
}
manager.ScanDlc();
tx.Complete();
}
manager.Save(enforce_consistency: false);
}

@Olympic1 Olympic1 removed the Bug Something is not working as intended label May 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core (ckan.dll) Issues affecting the core part of CKAN Registry Issues affecting the registry
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants