Wrong parsing of an application name containing "." (a dot) leads to a non-working AppImage

[Fr3nchK1ss]

Steps to reproduce:

1. Create an AppDir containing usr/bin/my_application_2.0_linux64 and usr/share/applications/my_application.desktop
   => my_application_2.0_linux64 is actually the name of the executable.

2. Run the following command line in a shell:
   ./linuxdeployqt-continuous-x86_64.AppImage AppDir/usr/share/applications/my_application.desktop -appimage -bundle-non-qt-libs -verbose=2

• You will notice that the AppDir was filled properly

• The AppImage was created without error

• But AppDir/AppRun is a symlink to AppDir/bin/my_application2

> ls -la AppDir
21 May  7 23:09 AppRun -> usr/bin/my_application2

that is to say the application name was truncated at the dot (parsing error). Trying to run the AppImage yields the following message :
execv error: No such file or directory

Work around

Rename the application binary file without any dot, for example my_application_2_linux64

[TheAssassin]

Oh well, this should not happen. I bet it's due to calling QFile(Info)'s baseName instead of fileName somewhere. They define both terms differently: in Qt's case, fileName returns the complete file name (not the full path), and baseName returns the file name without the file extension (which for them means, split by the first dot).

Should be an easy fix.

[probonopd]

Spot on... likely here:

linuxdeployqt/tools/linuxdeployqt/main.cpp

Line 196 in 63e768b

QString appName = QDir::cleanPath(QFileInfo(appBinaryPath).completeBaseName());

Thanks @TheAssassin - you are now a true master of Qt, debugging code without even looking at it ;-)

[TheAssassin]

Not really a master, but I have some experience with its QFileInfo class. Also, I have read a large portion of the Qt docs.

[tresf]

Would there be any reconsideration to add back in a check for the .real extension? It was proposed to use when we worked with @probonopd to originally create our first AppImage, but this patch reverts that option for us. More context is here: LMMS/lmms#4515 (comment)

[probonopd]

How would a proper fix look like?

[tresf]

How would a proper fix look like?

- QString appName = QDir::cleanPath(QFileInfo(appBinaryPath).completeBaseName());
+ // Assume ".real" is an attempt to shim additional runtime behavior, e.g. LD_LIBRARY_PATH
+ // take the basename instead
+ QString appName = appBinaryPath.endsWith(".real") ? 
+   QDir::cleanPath(QFileInfo(appBinaryPath).completeBaseName()) :
+   QDir::cleanPath(QFileInfo(appBinaryPath).fileName());

Probably worth putting in the official documentation if this is accepted. I looked around and couldn't find it anywhere except a note from you in our bug report...

probonopd said Jul 8, 2017 - If you urgently need the [...] environment variable, then move usr/bin/[foo] to usr/bin/[foo].real and put a bash script in its place that exports the variable and launches the real executable.

If we're simply doing it wrong, let us know that too. :)

[probonopd]

Well, hardcoding one particular extension, .real, is not a good idea imho. You could just as well just append some string ("_real" or another) without a dot as the filename for the "real" binary.

[tresf]

Well, hardcoding one particular extension, .real, is not a good idea imho. We could just as well just append some string ("_real" or another) without a dot.

I'm not sure what you mean. If this is an officially supported technique, you should set the standard, as this is your library. I was hesitant to even write it, TBH since this was a proposal by exactly you. :D

Whatever you decide, if you decide, let us know so that we can continuing using .AppImage for our project. If we're doing it wrong, we're happy to know that too. 👍

[tresf]

So some additional background on all of this...

• When we started integrating linuxdeployqt it was strongly encouraged to use the .desktop file as the source of the bundle -- but -- we found it didn't work properly with custom Exec= commands, e.g. ones that touch the environment before launching. In our case, it was an environmental variable for QT that we needed to set (something we'd consider and edge-case).
• We used the foo.real shim and linuxdeployqt automagically found our binary and everything worked
• Then, later, as we started to hit very specific issues with the .AppImage format, such as non-relocatable JACK libraries and other shims such as specific, nested folders that weren't being added to the LD_LIBRARY_PATH, we continued using this shim and it's been an important part of how we launch our software.

So, the root of the issue was the first environmental variable -- but -- the need to add more has made this just-in-time-custom-environment much more important as we embrace .AppImage as our preferred portable format (as a benchmark, our current release has 16,000 downloads in 2 months -- we consider .AppImage a success).

So what we need is the ability to shim custom logic into the launcher from the AppRun and it doesn't need to be obscure, we just need to know how we should tackle this problem. Although it started with an edge-case, the need to keep custom launch logic in some form -- I would expect -- is much more widely required by other projects.

We'd be happy to provide this as a supplement to the .desktop specification (e.g. ExecAppImage=foo.shim) but since we're still volatile (#227) to a Continuous release, we can't release until we have a viable solution.

[probonopd]

Maybe I am seeing it too simple, but it seems that a dot (.) in the Exec= value breaks it. So what I am suggesting is not to use a dot anymore. For example, have Exec=launcher, and have usr/bin/launcher as a shell script that sets the environment variables as needed and launches the main application.

Am I missing something?

[tresf]

Maybe I am seeing it too simple, but it seems that a dot (.) in the Exec= value breaks it.

"Break" being the operative word. I'll quote myself the first time this .real was proposed...

@tresf said: When we patch the desktop file, we change it to reference the Exec=lmms.real file. So... how does the AppImage know to create a symlink to /usr/bin/lmms, (the shell script) instead?

@tresf answered himself: [OK,] Internally, linuxdeployqt uses QFileInfo.completeBaseName() which strips off the extension per:

[Qt docs state] The complete base name consists of all characters in the file up to (but not including) the last '.' character.

@tresf said: So I guess this is intended behavior, which is nice. So I think this PR is ready to be merged. Note, it has not yet been added to Travis-CI.

Edit: It may be worth adding information about this shim to the documentation of linuxdeployqt.

So, I was confused by this behavior, pointed it out, but since upstream recommended it, I used it. Now it's been removed and we have no fallback option.

Perhaps we just need to stop using linuxdeployqt for anything launcher related. How do other projects shim last-minute stuff?

[probonopd]

@tresf again, what is forcing you to use a dot (.) in the Exec= value? Just that I did this in an example I made up earlier, when a dot in the Exec= value was still working?

The easy workaround is to just not use a dot (.) in the Exec= value, e.g., by substituting it with an underscore (_).

[tresf]

The easy workaround is to just not use a dot

Perhaps you are misunderstanding the bug and the resolution. The dot in the filename is what allowed the shim script to work in the first place. Adding an underscore does nothing because it was the behavior of the baseName that allowed the shim in the first place as it allowed the binary to have a different path than the launch command without breaking the automatic bundling behavior of the tool.

[probonopd]

Ah right, I forgot about how that script works. Can we change the shim script to use a _ rather than .?

[tresf]

linuxdeployqt reads the Exec= line and then does it's bundling based on that.

• Previously, if someone were to move /usr/bin/foo to /usr/bin/foo.real and specify Exec=foo.real, the AppRun would automatically drop off the .real extension and point to the shim located at /usr/bin/foo. Conveniently, the tool still would actually bundle dependencies of the executable foo.real but when launching the AppImage, would launch foo, due to the dropping of the file extension, allowing a shim script in the executable's place that handles just-in-time shimming.
• Now, if someone were to specify Exec=foo.real, the tool actually bundles foo.real which is good, but the AppRun also points to foo.real, so the shim is unused.

Can we change the shim script to use a _ rather than .?

I feel we're talking in circles here. With your advice, we moved the binary and linuxdeployqt was ok with it because of this bug. If you have no intentions of adding shim support back in, we'll have to write our own.

[TheAssassin]

You might want to check out linuxdeploy and its Qt plugin.

[tresf]

Since we've switched to using appimagetool, it looks like we can just fix this symlink ourselves before the call to appimagetool.

# After calling linuxdeployqt, point the AppRun to the shim launcher, not the foo.real version
ln -sfr [appdir]/usr/bin/foo [appdir]/AppRun

[TheAssassin]

@tresf linuxdeploy for instance respects existing AppRuns, and doesn't overwrite them.

[tresf]

Thanks, we'll eventually switch to that if it's the eventual successor to linuxdeployqt. When we first integrated with linuxdeployqt we didn't use appimagetool at all so we just used the .real trick as proposed. Since then we've gotten deeper into the workaround and have developed some fine-grained needs, so we're in good shape just making the AppRun ourselves after linuxdeployqt is done doing its thing. I like the idea of rolling our own AppRun script and not using a symlink. We'll eventually move to that after we get our next major release out the door.

[TheAssassin]

Cool. Feel free to visit #appimage on Freenode if you have questions.