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

Jump to bottom

Create ~/.local/share/applications/ if it doesn't exist (Linux) #1848

Merged
merged 2 commits into from
Dec 29, 2019
Merged

Create ~/.local/share/applications/ if it doesn't exist (Linux) #1848

merged 2 commits into from
Dec 29, 2019

Conversation

DinCahill
Copy link
Member

Discovered this on Ubuntu Server 16.04, where I didn't have this directory.

Directory.CreateDirectory creates directories recursively, so it should be fine.

@politas
Copy link
Member

politas commented Aug 1, 2016

Is there a new location for the .desktop files?

@DinCahill
Copy link
Member Author

I moved the CreateDirectory up to the constructor, and removed a test for the folder existing.

From the XDG spec:

If, when attempting to write a file, the destination directory is non-existant an attempt should be made to create it with permission 0700.

I'm not immediately sure how we should chmod 0700, so that still needs doing.

@Postremus
Copy link
Contributor

well, in the autoupdater, we literally just do a chmod +x.
https://github.com/KSP-CKAN/CKAN/blob/master/Core/Net/AutoUpdate.cs#L188

@politas
Copy link
Member

politas commented Aug 1, 2016

Can we test that the directory creation was successful and throw an error if not?

@DinCahill
Copy link
Member Author

@Postremus I'll copy that code, then.

ayan4m1
ayan4m1 reviewed Aug 2, 2016
View reviewed changes
GUI/URLHandlers.cs
string XDGDataHome = Environment.GetEnvironmentVariable("XDG_DATA_HOME");
if (XDGDataHome != null)
{
ApplicationsPath = Path.Combine(XDGDataHome, "applications");
Copy link
Contributor

Choose a reason for hiding this comment

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

The applications subdirectory is deprecated. The preferred path is specced here

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, thanks.

@DinCahill
Copy link
Member Author

So, we need a strategy for determining which mimeapps.list location to use. Link to the spec.

Should we write to the first one we find out of $XDG_CONFIG_HOME/mimeapps.list and $XDG_DATA_HOME/applications/mimeapps.list? If neither exists, which should we use? I don't know how old your distro would have to be to not support the one in $XDG_CONFIG_HOME.

I think $XDG_DATA_HOME/applications/ (falling back to ~/.local/share/applications/) is still the correct place for the .desktop file.

@politas
Copy link
Member

politas commented Aug 2, 2016

If neither location contains a mimeapps.list, we could possibly assume that the distribution is not supporting mime types, so why bother creating mimeapps.list?

@DinCahill
Copy link
Member Author

It doesn't mean mimeapps.list isn't supported. It just means that nothing's created any user-level mime-type associations yet. It's possible that a user's home directory could be completely empty, so we need to create these directories.

@politas
Copy link
Member

politas commented Aug 2, 2016

Hmm. Reading that spec, it sounds like we probably don't really need to be messing with mimeapps.list at all. We can put our associations in our ckan-handler.desktop file (which we should probably fix to be just ckan.desktop). mimeapps.list is to add or remove associations in the .desktop files. Am I off base?

@politas
Copy link
Member

politas commented Aug 2, 2016

Ok, I suppose that could be possible if people have a dedicated machine for running KSP.

@DinCahill
Copy link
Member Author

I think the distinction is that ckan-handler.desktop defines that it supports x-scheme-handler/ckan (for informational purposes), but mimetypes.list is used to actually make the association for URL opening.

If I remove (or delete the line from) mimetypes.list, I can't open links. If I instead remove MimeType=x-scheme-handler from ckan-handler.desktop, it still works.

I've just realized that this makes "CKAN Launcher" show up in application launchers, unsurprisingly, and it should work fine. Its category is "Utility" (comes up under Accessories in XFCE4). The icon is defined as "ckan" but I don't have any of the icons installed (they go in ~/.local/share/icons or something).

@politas
Copy link
Member

politas commented Aug 2, 2016

Link to spec for .desktop files. See the "Registering Mime Types" page.

@politas
Copy link
Member

politas commented Aug 2, 2016

I actually made my own CKAN.desktop launcher, just so I could take control of where I'm launching it from and which category it goes in.

@politas
Copy link
Member

politas commented Aug 2, 2016

It's almost like Ubuntu is not properly following the Freedesktop standards.

@DinCahill
Copy link
Member Author

It says that .desktop files simply define the mime-types they support. It's not enough for actually opening the URLs.

@politas
Copy link
Member

politas commented Aug 2, 2016

Fair enough.

@DinCahill DinCahill added the In progress We're still working on this label Aug 4, 2016
@DinCahill DinCahill self-assigned this Aug 4, 2016
@politas
Copy link
Member

politas commented Aug 4, 2016

$XDG_CONFIG_HOME/mimeapps.list seems like where it should go to me. That should be either the same as $XDG_DATA_HOME/applications/mimeapps.list or the correct location for the distribution. If $XDG_CONFIG_HOME is null, fall back to ~/.local/share/applications/

@Olympic1 Olympic1 added the Linux Issues specific for Linux label Sep 11, 2016
@politas politas mentioned this pull request Sep 16, 2016
Closed
ayan4m1
ayan4m1 reviewed Sep 27, 2017
View reviewed changes
GUI/URLHandlers.cs
@@ -159,13 +168,6 @@ private static void RegisterURLHandler_Linux()
parser.WriteFile(MimeAppsListPath, data);

var handlerPath = Path.Combine(ApplicationsPath, HandlerFileName);
var handlerDirectory = Path.GetDirectoryName(handlerPath);
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the rationale behind removing this code?

@ayan4m1 ayan4m1 self-assigned this Sep 27, 2017
@ayan4m1
Copy link
Contributor

ayan4m1 commented Sep 27, 2017

Just want to confirm why there was some code removed, otherwise LGTM.

@HebaruSan HebaruSan mentioned this pull request Dec 28, 2018
Closed
politas added a commit that referenced this pull request Dec 29, 2019
@politas
Merge #1848 Create ~/.local/share/applications/ if it doesn't exist
515934b
@politas politas merged commit 6bb86bf into KSP-CKAN:master Dec 29, 2019
@politas politas removed the In progress We're still working on this label Dec 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ayan4m1 ayan4m1 ayan4m1 left review comments

@dannydi12 dannydi12 dannydi12 approved these changes

Assignees

@ayan4m1 ayan4m1

@DinCahill DinCahill

Labels
Linux Issues specific for Linux
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@DinCahill @politas @Postremus @ayan4m1 @dannydi12 @pjf @Olympic1