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

Add APPIMAGETOOL_APP_NAME environment variable #18

Merged
merged 1 commit into from
Jul 5, 2023
Merged

Conversation

TheAssassin
Copy link
Member

Allows using the output name generation appimagetool provides with desktop files whose Name entry is unsuitable for the purpose. Users can specify their own prefix with this new environment variable.

Based on #17, please do not merge but only approve if it works for you. You can find the diff to the other branch here: https://github.com/AppImage/appimagetool/compare/document-env-vars...app-name-prefix?expand=1

@TheAssassin TheAssassin requested a review from probonopd July 5, 2023 13:59
Copy link
Member

@probonopd probonopd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion, but it works as designed and I'd prefer not to introduce this complexity. I can only see downsides in this - AppImage files with names different from how the application actually is named.

I consider it a feature that the Name= entry is used for naming the AppImage file, because only this way the two can guaranteed to be always consistent. The AppImage file should be named like the application is named, in the same spelling, capitalization etc. (just blanks replaced by _).

There should be only one "source of truth" for the name of the application. Don't like it? Change the Name= in the desktop file.

@TheAssassin
Copy link
Member Author

TheAssassin commented Jul 5, 2023

it works as designed

Nobody ever questioned that.

I can only see downsides in this - AppImage files with names different from how the application actually is named.

There is no logic in this argument. There already is an output path parameter that does exactly this.

I consider it a feature that the Name= entry is used for naming the AppImage file, because only this way the two can guaranteed to be always consistent. The AppImage file should be named like the application is named, in the same spelling, capitalization etc. (just blanks replaced by _).

Still there as a fallback.

There should be only one "source of truth" for the name of the application. Don't like it? Change the Name= in the desktop file.

That again doesn't make a lot of sense.


The rationale for this change is tools like appimagecraft or KDE craft which need to have predictable output. They usually know the application name, and want a unified name pattern on all platforms. It makes no sense to have an AppImage called My_Fantastic_Application-v1.2.3-x86_64.AppImage, whereas the Windows installer is called my-fantastic-application-v1.2.3.exe and the macOS binary is named similarly.

The reason why the output name parameter is not used (or rather useless) is that the automatic generation of a suitable name is actually pretty useful. Having appimagetool guess the architecture (which typically works quite well) and insert the version number in a standardized way is a good thing.

People should really be able to use this feature with a customized name and should never have to reproduce the name generation appimagetool already provides yet again. It makes no sense.

@TheAssassin
Copy link
Member Author

Also, having to actively rename the file makes no sense, just because the tool is limited in this way...

@TheAssassin
Copy link
Member Author

complexity

Have you actually seen the change? It doesn't add any significant complexity while fixing some potential stack overflows even...

@probonopd
Copy link
Member

The rationale for this change is tools like appimagecraft or KDE craft which need to have predictable output. They usually know the application name, and want a unified name pattern on all platforms. It makes no sense to have an AppImage called My_Fantastic_Application-v1.2.3-x86_64.AppImage, whereas the Windows installer is called my-fantastic-application-v1.2.3.exe and the macOS binary is named similarly.

And that is exactly my point. If the application is called "My Fantastic Application", then traditionally you have

  • My Fantastic Application.dmg for the Mac
  • my-fantastic-application.deb for Linux
  • AppImages shall be named in the correct uppercase/lowercase like on the Mac. Because unlike with packages (which are usually handled by a package manager), AppImages are supposed to be used by people in the file manager. It always drives me nuts to see AppImages all lowercase when in fact the application name is uppercase. Each time I see a lowercase AppImage when the application name is in fact uppercase, I think "why on earth do I need to buy a Mac just to get the correct spelling - what is it about Linux users to butcher product names".

But hey. If KDE Craft needs it, then well.

Also, having to actively rename the file makes no sense, just because the tool is limited in this way...

Renaming the files is a big no-no in any case, because it breaks AppImageUpdate.

@TheAssassin
Copy link
Member Author

More use cases:

  • https://github.com/pop-os/popsicle usually uses "USB Flasher" as a Name (a bit too generic outside of their OS, so the appimagecraft script changes it to "Popsicle USB Flasher". The AppImage's name becomes quite long this way. popsicle-v1.2.3-x86_64.AppImage would work better.
  • Nextcloud Desktop is called Nextcloud-3.9.0-x86_64.AppImage. The desktop entry provides "Nextcloud Desktop" as a name. They obviously chose to deviate from it, and therefore either have to manually rename the file or generate the full filename themselves.

Software, especially cross-platform and/or commercial, need such features.

@TheAssassin
Copy link
Member Author

because it breaks AppImageUpdate

I disagree. This only happens in some use cases. For instance, many projects generate their own .zsync files. Being able to do so from appimagetool is just a convenience feature. Also, these files can be patched yet again.

You really should see some of the deployment pipelines I came across in the last years...

@probonopd
Copy link
Member

probonopd commented Jul 5, 2023

https://github.com/pop-os/popsicle usually uses "USB Flasher" as a Name

Bad, bad, bad.

https://github.com/pop-os/popsicle calls it "Popsicle" in the README.md. That is the name of the product. So it should be Name=Popsicle, GenericName=USB Flasher, and Popsicle-0.0.0-x86_64.AppImage.

Maybe I am so picky about this because part of my dayjob is to enforce unified spelling of product names all the time.

But again, my thoughts here are not a hard veto. Just food for thought.

@TheAssassin
Copy link
Member Author

So what? It's the UX they chose. On their own desktop, this is perfectly fine (GNOME for instance just uses "Software" instead of "GNOME Software", "Files" instead of "Nautilus", etc., which works on such a desktop and of course is confusing outside of it). But I think this is not the place to discuss such things. I just want to show that these use cases exist and appimagetool could be more helpful.

@TheAssassin
Copy link
Member Author

not a hard veto

Well, you blocked merging with your changes request. Please approve the changes then.

@probonopd
Copy link
Member

probonopd commented Jul 5, 2023

You want to make it easy for them to continue that mess whereas I want to "forcefully remind" them to end the mess. ;-)

If you want to do me a favor, let's state in the documentation for this environment variable that it is best practice to use the exact same name, spelling, and upper/lowercase for:

  • README
  • Name=
  • AppImage file name
    because consistent naming of an application makes it more professional.

Allows using the output name generation appimagetool provides with desktop files whose `Name` entry is unsuitable for the purpose. Users can specify their own prefix with this new environment variable.
@TheAssassin TheAssassin marked this pull request as ready for review July 5, 2023 22:59
@TheAssassin TheAssassin merged commit 62ed643 into main Jul 5, 2023
@TheAssassin TheAssassin deleted the app-name-prefix branch July 5, 2023 23:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants