flathub-infra / flatpak-external-data-checker

A tool for checking if the external data used in Flatpak manifests is still up to date
GNU General Public License v2.0
115 stars 34 forks source link

Print a more helpful error message if manifest path is not writable #390

Closed bbhtt closed 11 months ago

bbhtt commented 11 months ago

See https://github.com/flathub/org.flathub.flatpak-external-data-checker/issues/117

I'm not convinced it is a good idea to print custom messages than the inbuilt ones...

The other places I saw a file was being opened as writable is in add_release_to_file and dump_manifest. I don't think the appdata can get updated independently of the manifest, so both should be handled by this.

Some of the git operations might fail due to lack of permissions/whatever but I haven't checked for those as they have no error handling.

Eg. try committing changes from the flatpaked version without access to the git's config file (if it in xdg-config/git, not handled by --filesystem=host) - it prints an ugly error message. Try committing changes without any actual change - same thing.

dreua commented 11 months ago

I was thinking about an even more concrete hint like:

Run the app with flatpak run --filesystem=$(pwd) org.flathub.flatpak-external-data-checker [args]

The use case is to educate people who are not familiar with the flatpak CLI or just forget stuff they used to know while still having all the benefits of sandboxing.

dreua commented 11 months ago

I now see the issue with git and it's config files, that might indeed be a problem. In GUI apps we need and have some defaults too which get passed in the sandbox to get everything to work but it is probably not as mature for CLI. On the other hand, when I last used this the above hint really helped me and was sufficient.

bbhtt commented 11 months ago

I was thinking about an even more concrete hint like:

That won't work. You can be running the checker in a different working directory while having the manifest in a separate directory. Then --filesystem=$(pwd) would resolve to the current working directory, not the location of the manifest.

Eg. try running it in ~/Downloads while having the manifest not in ~/Downloads or any of its subdirectory like in ~/Projects.

dreua commented 11 months ago

I know that. The use case is a developer with some experience in Linux who understands stuff like that but just doesnt know or think about the faltpak specific necessities and commands.

bbhtt commented 11 months ago

Added a bit better hint.

wjt commented 11 months ago

I think it's better to just let it write to whatever it can see. I will merge https://github.com/flathub/org.flathub.flatpak-external-data-checker/pull/223.