Skip to content
This repository has been archived by the owner on Oct 3, 2020. It is now read-only.

Add option to specify directories to watch #74

Closed
wants to merge 1 commit into from
Closed

Add option to specify directories to watch #74

wants to merge 1 commit into from

Conversation

genderquery
Copy link
Contributor

Adds a check for the APPIMAGED_WATCH_DIRS environment variable. If
present, appimaged will watch the colon (:) separated list of
directories instead of the default.

If using systemd, the user can create
$HOME/.config/systemd/user/appimaged.service.d/10-watch-dirs.conf as a
drop-in for appimaged.service with something similar to the following
contents:

[Service]
Environment=APPIMAGED_WATCH_DIRS=/home/avery/Applications:/Applications

Note that systemd will not perform variable expansion, so you can't
specify something like $HOME/Applications, unfortunately.

In the current implementation, if using APPIMAGED_WATCH_DIRS, the check
for /Applications on all mounts will not be performed.

I wrapped g_getenv("APPIMAGE") in a call to g_strdup() because the
GLib documentation states, "The returned string may be overwritten by
the next call to g_getenv(), g_setenv() or g_unsetenv()." I didn't want
to risk my call to g_getenv() invalidating the reference to
appimage_location. I also later freed it after it was used.

May resolve issues #29 and #44.

I'm not sure if this is the best approach, but I didn't want to introduce complexity with configuration files.

Adds a check for the APPIMAGED_WATCH_DIRS environment variable. If
present, appimaged will watch the colon (:) separated list of
directories instead of the default.

If using systemd, the user can create
`$HOME/.config/systemd/user/appimaged.service.d/10-watch-dirs.conf` as a
drop-in for appimaged.service with something similar to the following
contents:

```
[Service]
Environment=APPIMAGED_WATCH_DIRS=/home/avery/Applications:/Applications
```

Note that systemd will not perform variable expansion, so you can't
specify something like `$HOME/Applications`, unfortunately.

In the current implementation, if using APPIMAGED_WATCH_DIRS, the check
for /Applications on all mounts will not be performed.

I wrapped `g_getenv("APPIMAGE")` in a call to `g_strdup()` because the
GLib documentation states, "The returned string may be overwritten by
the next call to g_getenv(), g_setenv() or g_unsetenv()." I didn't want
to risk my call to `g_getenv()` invalidating the reference to
`appimage_location`. I also later freed it after it was used.
@probonopd
Copy link
Member

Thanks @genderquery. We are trying to do without configurability where we can, in order to make things simple, reproducible, easy to explain/document, and avoid differences between user's systems.

Can you give an example where the provided locations are not sufficient?

@genderquery
Copy link
Contributor Author

That makes sense @probonopd. I wrote this for my own purposes and figured I'd share in case it was useful and I understand if you'd rather not merge it.

To remind those following this conversation, appimaged currently looks in the following locations:

  • $HOME/.local/bin
  • $HOME/Downloads
  • $HOME/bin
  • $HOME/.bin
  • $HOME/Applications
  • /Applications
  • /opt
  • /usr/local/bin

On my system, my $HOME/Downloads directory is quite large with many files in it. When I initially started appimaged, my computer slowed dramatically as it attempted to index this directory.

Thank you for taking the time to review my pull request.

@probonopd
Copy link
Member

When I initially started appimaged, my computer slowed dramatically as it attempted to index this directory.

True, I think this is what we need to tackle first and foremost. Do you have ideas how to make the directory scanning more efficient? One way to achieve this would be to only use AppImages that have the .AppImage filename extension. I think that should allow for a considerable speed-up. But would it be worth the trade-off?

@genderquery
Copy link
Contributor Author

Looking at the file extension would help considerably, however this would run counter to the current draft of the AppImage Specification which states that an AppImage "MUST NOT rely on any specific file name extension".

I'll think on it more and if I come up with anything, I'll open a new issue or make a pull request for it. Feel free to close this PR. Thanks for taking the time to review it and offer feedback :-)

@probonopd
Copy link
Member

an AppImage "MUST NOT rely on any specific file name extension"

This first and foremost means that the AppImage still must work when it is renamed or moved (not necessarily the appimaged desktop integration experience although it would be nice). If the appimaged desktop integration experience can only be improved (to the level of how Mac apps "just work") by using a certain filename extension, I might consider it.

@probonopd probonopd closed this Mar 18, 2019
@is-this-a-comma-or-period

Hi, I am not sure where to put this at all, as the issues like #29 are closed.

I believe that it should be possible to stop registering a certain AppImage by renaming it, like "Emacs-26.3-x86_64.AppImage_disabled" is literally telling a human being that ... this AppImage file is disabled. (In this case, because I compiled Emacs manually. I decided to keep the downloaded AppImage file just in case. But it appears in my Cinnamon Desktop menu. I am a keyboard junky and more, I don't want to have distractions when calling a/any program.)

Alternatively I could move the file from my ~/Downloads/... directory, but that would be very unnatural, to modify directory structures that have been proven OK for years. Just not to have an entry in my launcher menu.

I could use that environment variable APPIMAGED_WATCH_DIRS but this would overwrite all the presets that you or the project made with considerations. I could manually copy that list and adjust it.

But this would overwrite your thoughtful selection of paths (a single entry just does not fit my needs), and if you adjusted this path selection in a future release, I had to adjust my configuration - but only after mentally registering that change.

I would suggest 2+0,5+0,5 things out of this:

  • accept that someone wants to disable an AppImage file by not registering it
    • use a signal word/string/suffix like .disabled
    • option to search only for files ending with AppImage, containing AppImage (APPIMAGED_EXTENSIONS=* or =.AppImage:.ThatsSoCool or APPIMAGED_SEARCH=<any|strict|disabled>) (that's all only a quick draft)
  • use exclude option APPIMAGED_EXCLUDE_DIRS . Because I know best which directories are off-limits and you know best where AppImage files could be placed.

The user installing AppImaged will always have the officially recommended search paths with every release.

In the first case, as you stated, you would speed up the search and stop weird cases like VBox image files. And, honestly, as a user I want to be able to disable things with at least effort possible. Calling a thing ".disabled" should be a hint.

@probonopd
Copy link
Member

probonopd commented Jun 27, 2020

Thanks for your thoughtful considerations @is-this-a-comma-or-period. I do like the .disabled idea because it is explicit, and it does not require configuration. Could also add .~, .backup to the list of suffxes that disable desktop integration. Would you like to send a pull request that would implement this?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants