search

ximion / appstream-generator

A fast AppStream metadata generator
GNU Lesser General Public License v3.0
44 stars 29 forks source link

FreeBSD backend #113

Closed arrowd closed 1 year ago

ximion commented 1 year ago

Some Linux distributions as well as FreeBSD OS install 3rd party software into a prefix that differs from "/usr". This change makes asgen work in such cases.

Can you elaborate on that? Installing metadata into any location outside of /usr/(local)/share/metainfo is not permitted and not supported by any other tool, so I would really not like to make an exception here...

ximion commented 1 year ago

Also, by changing this you would not catch all system software that apparently is installed in /usr, and by making this a global build-time flag you can not process data for any other system other than FreeBSD correctly. So you can't use appstream-generator on FreeBSD to process a Debian or Arch Linux archive (or vice versa).

arrowd commented 1 year ago

Can you elaborate on that?

FreeBSD installs 3rdpart software into /usr/local. Some Linuxes, AFAIK, install stuff into /opt.

Also, by changing this you would not catch all system software that apparently is installed in /usr

I was under assumption that there could be no system software that have AppStream metadata. At least there are no in FreeBSD. But this point is valid.

and by making this a global build-time flag you can not process data for any other system other than FreeBSD correctly. So you can't use appstream-generator on FreeBSD to process a Debian or Arch Linux archive (or vice versa).

I didn't even think about such use case. Yes, this is a valid point too.

I'm happy to fix this, but how? For FreeBSD backend to work I need asgen to use /usr/local prefix everywhere and without the first commit of this PR it is not possible.

ximion commented 1 year ago

FreeBSD installs 3rdpart software into /usr/local. Some Linuxes, AFAIK, install stuff into /opt.

But do FreeBSD packages also contain stuff in /usr/local? /opt is not a supported location at all for AppStream metadata, /usr/local/share/metainfo is fairly uncommon for packaged stuff, but that location is at least read, so we could potentially also support it.

ximion commented 1 year ago

Can you change this patch to include just the backend without the prefix changes? That will make it much easier to review, as the prefix stuff will need a lot more work (and thought).

arrowd commented 1 year ago

But do FreeBSD packages also contain stuff in /usr/local?

Yes, all of them. It can be configured on the builder machine, but the default is /usr/local.

Can you change this patch to include just the backend without the prefix changes? That will make it much easier to review, as the prefix stuff will need a lot more work (and thought).

Ok, will do.

arrowd commented 1 year ago

Bump. Can we get this in?

arrowd commented 1 year ago

Just a tiny nitpick about the TODO entry in the code, and the general disclaimer that I can't really test the code.

We're running it for about a month now, so it works for us.

ximion commented 1 year ago

We're running it for about a month now, so it works for us.

That's good enough - if the conflicts of this PR with the master branch are resolved, we can merge this (and maybe even remove the TODO, since it obviously works).

arrowd commented 1 year ago

This should be ready to get in now.

ximion commented 1 year ago

This should be ready to get in now.

It is indeed! Thank you for your patch and your patience!

arrowd commented 1 year ago

Thanks for merging it! We now still need two things to make it usable:

I'm happy to work on these issues myself, so please provide me some input on how to fix them.