After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 691841 - gvfs is not compatible with new separate libcdio-paranoia-0.90 package
gvfs is not compatible with new separate libcdio-paranoia-0.90 package
Status: RESOLVED FIXED
Product: gvfs
Classification: Core
Component: build
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gvfs-maint
gvfs-maint
Depends on:
Blocks:
 
 
Reported: 2013-01-16 11:06 UTC by Samuli Suominen
Modified: 2013-01-24 14:55 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
cdda: Adapt to new paranoia.h location (1.82 KB, patch)
2013-01-16 16:16 UTC, Tomas Bzatek
committed Details | Review

Description Samuli Suominen 2013-01-16 11:06:10 UTC
upstream applied this fix on top of 0.90 release (in their git):

--- include/cdio/paranoia/Makefile.am
+++ include/cdio/paranoia/Makefile.am
@@ -3,8 +3,8 @@
 ########################################################
 #
 
-libcdioincludedir=$(includedir)/cdio
-dist_libcdioinclude_HEADERS = cdda.h paranoia.h 
+cdio_paranoia_includedir=$(includedir)/cdio/paranoia
+dist_cdio_paranoia_include_HEADERS = cdda.h paranoia.h 
 
 EXTRA_DIST = version.h.in
 BUILT_SOURCES = version.h

this fixes the cdda.h and cdparanoia.h headers to be at cdio/paranoia/cdda.h and cdio/paranoia/paranoia.h

but daemon/gvfsbackendcdda.c has hardcoded paths for older libcdio releases where cdda.h and cdparanoia.h still used to be in cdio/ path

both Arch and Gentoo have gone through the migration, and according to Fedora ML they are doing it too

ideally would go to 1.14 and master

thanks
Comment 1 Samuli Suominen 2013-01-16 11:07:14 UTC
propably easy to accomplish this using AC_CHECK_HEADERS and #ifdef
Comment 2 Samuli Suominen 2013-01-16 13:03:35 UTC
the link to the commit that moves them over to cdio/paranoia/ is here:

https://github.com/rocky/libcdio-paranoia/commit/b2807f3c7a4126b6078d96adbd37c3760b9f41ab
Comment 3 Tomas Bzatek 2013-01-16 14:41:59 UTC
(In reply to comment #0)
> this fixes the cdda.h and cdparanoia.h headers to be at cdio/paranoia/cdda.h
> and cdio/paranoia/paranoia.h

Alright, libcdio-paranoia-0.90 indicates -I${includedir}/cdio (for /usr/include/cdio/paranoia/paranoia.h) in it's pkg-config file while older versions have just -I${includedir} (for /usr/include/cdio/paranoia.h). Rather incompatible change.

> but daemon/gvfsbackendcdda.c has hardcoded paths for older libcdio releases
> where cdda.h and cdparanoia.h still used to be in cdio/ path
> 
> both Arch and Gentoo have gone through the migration

Do you have an updated ebuild handy?

> and according to Fedora ML they are doing it too

Looking at Fedora libcdio-paranoia-devel-10.2+0.90-6, it manually copies both headers in /usr/include/cdio/ and /usr/include/cdio/paranoia/ for the moment:

# copy include files to an additional directory
# this will probably be the location for future releases see:
# https://github.com/rocky/libcdio-paranoia/commit/b2807f3c7a4126b6078d96adbd37c3760b9f41ab
Comment 4 Samuli Suominen 2013-01-16 15:01:26 UTC
(In reply to comment #3)
> (In reply to comment #0)
> > this fixes the cdda.h and cdparanoia.h headers to be at cdio/paranoia/cdda.h
> > and cdio/paranoia/paranoia.h
> 
> Alright, libcdio-paranoia-0.90 indicates -I${includedir}/cdio (for
> /usr/include/cdio/paranoia/paranoia.h) in it's pkg-config file while older
> versions have just -I${includedir} (for /usr/include/cdio/paranoia.h). Rather
> incompatible change.
> 
> > but daemon/gvfsbackendcdda.c has hardcoded paths for older libcdio releases
> > where cdda.h and cdparanoia.h still used to be in cdio/ path
> > 
> > both Arch and Gentoo have gone through the migration
> 
> Do you have an updated ebuild handy?
> 
> > and according to Fedora ML they are doing it too
> 
> Looking at Fedora libcdio-paranoia-devel-10.2+0.90-6, it manually copies both
> headers in /usr/include/cdio/ and /usr/include/cdio/paranoia/ for the moment:
> 
> # copy include files to an additional directory
> # this will probably be the location for future releases see:
> #
> https://github.com/rocky/libcdio-paranoia/commit/b2807f3c7a4126b6078d96adbd37c3760b9f41ab

I have gone through entire gentoo-x86 (portage tree) and converted every package to use the new directory structure cdio/paranoia/{cdda,paranoia}.h
I don't want any temporarily kludges that keep the files where they were since they were intentionally moved over to the paranoia/ subdir
I can see why Fedora is doing it, but I think it will only cause things getting missed, when I just want this over with
Comment 6 Tomas Bzatek 2013-01-16 16:16:32 UTC
Created attachment 233609 [details] [review]
cdda: Adapt to new paranoia.h location

(In reply to comment #1)
> propably easy to accomplish this using AC_CHECK_HEADERS and #ifdef

Yeah, I went this way. Since the change came post-0.90 release, checking pkg-config version may not be reflecting actual location.

(In reply to comment #4)
> I have gone through entire gentoo-x86 (portage tree) and converted every
> package to use the new directory structure cdio/paranoia/{cdda,paranoia}.h
> I don't want any temporarily kludges that keep the files where they were since
> they were intentionally moved over to the paranoia/ subdir
> I can see why Fedora is doing it, but I think it will only cause things getting
> missed, when I just want this over with

Of course, always better to stick with a clean solution and adapt the rest.

(In reply to comment #5)
> (In reply to comment #3)
> > > both Arch and Gentoo have gone through the migration
> > Do you have an updated ebuild handy?
> 
> http://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/dev-libs/libcdio-paranoia/files/libcdio-paranoia-0.90-headers.patch?view=markup
> http://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/dev-libs/libcdio-paranoia/libcdio-paranoia-0.90.ebuild?view=markup

Thanks, synced and emerged.


Attaching a possible patch for gvfs, please test.
Comment 7 Tomas Bzatek 2013-01-24 14:55:20 UTC
Pushed as c79426bf1f5745b7e3427958a0e6fb6e0bdefa93. Please shout if it breaks anything.