Mudlet Crashing When Using Package Exporter

[ImpZyv]

Brief summary of issue / Description of requested feature:

As the title says, for no apparent reason Mudlet will get the 'Mudlet has stopped working' window when attempting to export a package. The package will 'export' but it's just the config.lua, and a blank xml file, like so:

Steps to reproduce the issue / Reasons for adding feature:

1. Use package exporter
2. Select items
3. Hit export package

Error output / Expected result of feature

"Mudlet has stopped working"

Extra information, such as Mudlet version, operating system and ideas for how to solve / implement:

I got around it just by using the 'export' button, and 'packaging' things the old fashioned way... I'd prefer to use the package manager, as installation is much smoother that way... But it works for now, I guess.

Yes, I've tried running as admin. Yes, it's allowed through my firewall / anti-virus. Yes, it's still producing the error/crash after doing all of that.
Version 3.10.1
OS8.1

[SlySven]

It seems to me that you are NOT trying to include any other files (fonts, images, etc) in a sub-directory - that still doesn't work IIRC as it creates all the files needed in the "staging" are (a sub-directory with the created package name inside a /temp directory inside the base directory of the creating profile) that fails currently to add sub-directories to the archive file in the intended archive file and silently aborts...

In fact there is a number of missing error messages from the current package/module exporter code so something could have gone wrong and the user would not know about it. I had some fixes lined up (as PR #1355) but that grown too large, had stalled and then drifted off my radar... 🙁

Superficially it looks like the package exported code has failed to write the Mudlet item data into the .xml file - what worries me is that the code to save a profile was revised recently to save things asynchronously, if that code is also involved in the process of saving selected items as a module/package then we may have a problem in that we need the wanted things saved straight away without waiting so it can then be added to the archive (zip format) file that is the desired end product!

Is the file C:\Users\<userName>\.config\mudlet\profiles\<profileName>\temp\AtaxiaSystemPackage\AtaxiaSystemPackage.xml also zero length as well as the file in the AtaxiaSystemPackage.zip file?

[ImpZyv]

Not trying to include anything except for Aliases/Scripts/Triggers, no.

The files in the temp file are being 'exported' as per usual. 890kb, and seems to have all the stuff inside it that it's meant to; and the same stuff it had prior to 3.10. Only the stuff in the actual .zip file isn't being written correctly, it looks like.

[SlySven]

🤔 Yeah, what I am wondering is whether that .xml file has actually finished being written before the code that includes it in the .zip file is run - if not, it might be empty then but by the time you check it, the data has been flushed out to the file and shows up as expected...

... Lemme take a look.

[SlySven]

Oh dear, @vadi2 I do not grok the XMLexport code any more but it looks to me that the code that does the package/module export (void) dlgPackageExporter::slot_export_package() does not engage/use the new code correctly when it gets to the:

writer.exportGenericPackage(filePath);

line (210) as there is no indication that the code thethat uses the .xml file created by that call - which starts around line 264 in the same method to make a zip archive waits for the file to be written out to the file-system so that it can be accessed by the code using the libzip library there.

Of course - with the current lack of error detection/handling - there is also little/no provision to abort the latter if the former fails for any reason.

Setting this bug to high priority because it is causing crashes in the field - and because of the mechanism, (part of the code base that was not checked/fixed perhaps when another part that it used was changed) I guess this also makes it a regression.

I do not think I can fix it because I do not fully understand how the new asynchronous saving code works... 😢

[SlySven]

@ImpZyv actually, can you do a diff of the .xml in the tmp directory against the broken (?) one stored in the .zip file - I wonder if the latter is a truncated partial copy of the wanted file. Given that it is .xml it is something that you should be able to open it in a text editor. (If it were complete the FireFox browser, at least, can show you the contents - it may also work for an incomplete file or it may just moan loudly about where the first error is...)

[ImpZyv]

I have no idea what 'doing a diff' means. The xml in the 'actual' file has nothing in it, though. The xml file in the tmp folder has everything it's meant to, judging from when I opened it the first time.

[SlySven]

I guess you are not on a *nix-like OS - so would not be familiar with the diff(1) utility https://en.wikipedia.org/wiki/Diff. I was thinking there was a partial, but truncated file in the .zip file but indeed it looks like there is nothing at all - which is consistent with what I think is happening...

[ImpZyv]

W8.1!

[Kebap]

I can reproduce this issue on Win7. A zip file is written with contents: config.lua and test.xml (as I named my package test), both have a file size of 1kb and config.lua has just one line as may be expected. However test.xml has a very basic structure and does not have all my triggers, etc. inside which I did check mark in the aforementioned popup window before clicking "export" with no additional files included. Here are the xml file's contents:

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE MudletPackage>
<MudletPackage version="1.001">
	<TriggerPackage />
	<TimerPackage />
	<AliasPackage />
	<ActionPackage />
	<ScriptPackage />
	<KeyPackage />
	<VariablePackage>
		<HiddenVariables />
	</VariablePackage>
</MudletPackage>

[vadi2]

@ImpZyv @Kebap does https://ci.appveyor.com/api/buildjobs/w0asc718iaxqfnpv/artifacts/src%2Fmudlet.zip fix the issue?

[Kebap]

Following above instructions, after clicking "export" nothing seems to happen anymore. No Mudlet crash at least. Also no error/success messages, etc.

Checking the specified file directory (navigating there myself in a new window, as no link was pointing me there) I found the file with the given name. The contents seem to be as expected.
In a second test, I found the file not written as expected. It seems like not working, if a specified filename already exists. Again no error message, just failing silently. Then silent success with a different non-existing filename.
Not sure if this functionality was as-is before, as I never used that feature much before. Let's make a new issue if so. At least the crash disappeared for me in Win8.

[SlySven]

IIRC That is another Hindenbug in the module exporter code, I think it is caused by the zip_add call in the following code in dlgPackageExporter::slot_export_package() [this is from the current development version]:

    archive = zip_open(zipFile.toStdString().c_str(), ZIP_CREATE, &err);
    if (err != 0) {
        zip_error_to_str(buf, sizeof(buf), err, errno);
        //FIXME: report error to user qDebug()<<"dp zip open error"<<zipFile<<buf;
        close();
        return;
    }
    QDir dir(tempDir);
    QStringList contents = dir.entryList();
    for (int i = 0; i < contents.size(); i++) {
        QString fname = contents[i];
        if (fname == "." || fname == "..") {
            continue;
        }
        QString fullName = tempDir + "/" + contents[i];
        struct zip_source* s = zip_source_file(archive, fullName.toStdString().c_str(), 0, 0);
        if (s == nullptr) {
            int sep = 0;
            zip_error_get(archive, &err, &sep);
            zip_error_to_str(buf, sizeof(buf), err, errno);
            //FIXME: report error to userqDebug()<<"zip source error"<<fullName<<fname<<buf;
        }
        err = zip_add(archive, fname.toStdString().c_str(), s);
        if (err == -1) {
            int sep = 0;
            zip_error_get(archive, &err, &sep);
            zip_error_to_str(buf, sizeof(buf), err, errno);
            //FIXME: report error to userqDebug()<<"added file error"<<fullName<<fname<<buf;
        }
    }

zip_add is the obsolete (libzip 0.1x era) equivalent to the current zip_file_add with no flags - importantly it fails if the file is already present in the archive - in that situation in the past one was suppose to use zip_replace instead; nowadays it is permissible to use zip_file_add with the ZIP_FL_OVERWRITE flag which does what it says - permits overwriting of the same named file in the archive.

[vadi2]

Following above instructions, after clicking "export" nothing seems to happen anymore. No Mudlet crash at least. Also no error/success messages, etc.

That's how it worked before... not great but I'll leave the redesign of it to SlySven#4

It seems like not working, if a specified filename already exists

I'll adjust that given that @SlySven's already done the investigative work above!

[vadi2]

@Kebap does https://ci.appveyor.com/api/buildjobs/gaspbd6ehobx4ykj/artifacts/src%2Fmudlet.zip work with an already existing package ok?

[Kebap]

Testing this version on Win7, it did not crash, but it did not export properly. The test.xml file in the test.zip was created with 0kb size and no contents, even though I did check contents to export.

[vadi2]

Hm, you sure? It worked on Ubuntu and Win10. Could you test again?

[Kebap]

Retried with the same setup, received the same result.
Retried with Win8 setup, received expected success! :) edit: That is, existing file gets overwritten silently.

The above error may have to do with #1616 as my Win7 user has non-ASCII name.
The folder I exported to in Win7 also had non-ASCII path.
Did not test Win7 with strictly ASCII username / folder.

[vadi2]

Yes, ok. So this issue is fixed, some outstanding ones no.

[vadi2]

#1847 will address the non-ASCII path.