juhp / cabal-rpm

Haskell Cabal RPM packaging tool
https://hackage.haskell.org/package/cabal-rpm
GNU General Public License v3.0
25 stars 8 forks source link

Declare installed data files in the %files section. #39

Closed peti closed 8 years ago

peti commented 8 years ago

This is required on SUSE Linux for those builds to succeed.

Cc: @mimi1vx

juhp commented 8 years ago

BTW do you have some example package(s) you are testing this on?

peti commented 8 years ago

ghc-barrier is a package that's cheap to compile but has a somewhat involved directory structure of installed files.

peti commented 8 years ago

BTW, just pushed a trivial update to this PR to avoid generating duplicate %dir entries.

peti commented 8 years ago

One package where this mechanism fails is happy because that tool uses a custom build to extend the list of installed data files at configure time. We cannot easily detect this, obviously.

juhp commented 8 years ago

Okay I had a look over the changes - I think they look okay. But I am still not completely clear what problem you're trying to solve? I know SUSE likes to list all files explicitly... is that what you're trying to do here? If so I might well make this SUSE specific.

On a related know i have pondered to change to listing files explicitly in spec files rather than using filelists, but I will probably wait to do that until I am ready for ghc8...

peti commented 8 years ago

I know SUSE likes to list all files explicitly.

SUSE requires all installed files to be declared. Without this patch, cabal-rpm generates invalid (failing) builds for all packages that install data files.

juhp commented 8 years ago

Okay, so just listing directories is not sufficient for SUSE packages, right?

In Fedora if a binary package owns all files under a directory we generally prefer and tend just to list the directory. So I think I prefer to make this change SUSE specific for now at least.

peti commented 8 years ago

Okay, so just listing directories is not sufficient for SUSE packages, right?

It would be. We could simply add an entry for %{_datadir}/%{pkg_name}-%{version} instead of listing every file separately. It would simplify the code, no doubt. Not sure why I didn't go for that route. I guess I felt that being explicit is nice, i.e. since we know the list files that ought to be installed, we might as well list them. On the other hand, some custom build system extend the list of data files beyond what the cabal file specifies. In that case, the explicit spec file is wrong, whereas the spec file that just mentions the base directory would still work. It's probably a matter of taste what one prefers.

juhp commented 8 years ago

Alright, anyway you still want to have the complete listing for SUSE, right?

If so I can merge this and then make it SUSE specific: it will make maintenance a bit harder though.

peti commented 8 years ago

You still want to have the complete listing for SUSE, right?

It seems nicer that way but I don't feel strongly about it. Just do whatever you think is best.

juhp commented 8 years ago

I hope this commit could address the issue sufficiently for now.