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

Move to/delete from $HOME/.local/share transactionally (all at once) #28

Open
probonopd opened this issue Dec 7, 2018 · 9 comments
Open
Labels

Comments

@probonopd
Copy link
Member

probonopd commented Dec 7, 2018

Currently libappimage directly extracts e.g., desktop files directly to $HOME/.local/share (or deletes them from) on each invocation of

  • appimage_type1_register_in_system
  • appimage_type2_register_in_system
  • appimage_unregister_in_system

Each change in /.local/share can trigger local caches such as Sycoca to be re-generated, which results in a lot of CPU activity.

So we should also apply the same logic that @rszibele used in AppImageCommunity/appimaged#50 to all files (desktop files, icon files, mime type files and such) that get copied to HOME/.local/share for system integration.

We should only atomically (=everything in one go) move files to (from a temporary location to which they get extracted and modified in) or delete files from $HOME/.local/share after a certain timer of inactivity has passed.

We should do all editing/rewriting of desktop files outside of HOME/.local/share in a separate directory.

This will solve:

@TheAssassin
Copy link
Member

Please don't use the term "atomic", it doesn't quite fit here. What you want to do is to move the files "transactionally", i.e., prepare all the move commands and then execute them at once (possibly aborting/reverting if one operation fails).

@probonopd
Copy link
Member Author

"All at once" is what I mean, yes.

@probonopd probonopd changed the title Atomically move to/delete from $HOME/.local/share Move to/delete from $HOME/.local/share transactionally (all at once) Dec 7, 2018
@TheAssassin
Copy link
Member

I guess the new libappimage C++ interface could receive a "BatchDesktopIntegrator" which allows for registering AppImages for integration, and a exec() method that performs the integration. We could then provide a DesktopIntegrator wrapper for this which creates an instance of BatchDesktopIntegrator, registers the AppImage in question, and runs the integrator.

@probonopd
Copy link
Member Author

The library should have a timer. Calling things like appimage_type1_register_in_system() should return immediately but not actually perform the integration, but merely write to /tmp. Once the timer expires (no more writes went to the temporary location in the last X seconds), everyhting would be copied in at once.

Makes sense?

@TheAssassin
Copy link
Member

No, all these implicitnesses are actually really bad design. These kinds of usages are something to be handled on a higher level, as they don't fit every use case (only appimaged could make use of this, but for e.g., AppImageLauncher, we want a prompt result, and the application might even exit before that timer expires...). appimagelauncherd for instance handles this just fine on such a higher level, but could definitely benefit from a batch integrator.

@rszibele
Copy link

rszibele commented Dec 7, 2018

I was thinking about adding this in libappimage as I implemented that functionality in appimaged, but I came to the same conclusion that @TheAssassin mentioned, that having that functionality in appimage_register_in_system would be a bad idea.

Calling appimage_register_in_system and the unregister equivalent should behave exactly as one would expect it to behave (as it does now). Extra functionality should be built on top of it through a batching interface.

My additions to appimaged could then be re-purposed to use the batching mechanism instead of calling appimage_register_in_system for each AppImage. However, the timer is something that should be implemented in the daemons themselves.

@probonopd
Copy link
Member Author

probonopd commented Dec 7, 2018

How can appimaged handle these things when actually all the work happens in appimage_typeX_register_in_system() which is in libappimage?

Wouldn't that mean that appimaged would have to implement its own appimage_typeX_register_in_system(), making libappimage redundant?

@azubieta
Copy link
Contributor

azubieta commented Mar 8, 2019

@rszibele @probonopd this could be done by setting a temporary HOME while calling appimage_register_in_system and then moving all at once. But I do agreed that it would better fit into libappimage.

@probonopd
Copy link
Member Author

Yes, in libappimage please.

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

No branches or pull requests

4 participants