(In reply to comment #0)
> The rules files that are installed in /usr/share/tracker/extract-rules are not
> architecture-neutral, since they include a full path to the module, like
> 
> /usr/lib64/tracker-0.16/extract-modules/libextract-gstreamer.so
> 
> This causes problem for multilib installations.
> 
> I would suggest to simply allow relative paths here, and prepend
> 
> ${libdir}/tracker-0.16/extract-modules 
> 
> if the ModulePath does not start with a /

Hi Matthias, just so I understand:

Do you mean the content of the .rules files should be something like:

  ModulePath=../../../lib64/tracker-0.16/extract-modules/libextract-gstreamer.so

and at runtime we use a simple check like:

  if (g_str_has_prefix(modulepath, "/")) {
    modulepath = g_build_filename(libdir, modulepath, NULL);
  }

perhaps?

--

Also, we currently use ${libdir} at build time in Makefile.am and I wonder why this itself doesn't point to the right place (/usr/lib64) already? From what I've researched, /usr/lib should be used over /usr/lib64 anyway - which raises some questions here (unless it's specifically stated with autogen.sh or configure of course).

Finally, would you prefer the prefix for ModulePath to be configurable at build time instead of relying on ${libdir} as it is now using something like ./configure --foo-dir=/bar?

Thanks :)

No, I would suggest that the rule file just contains

ModulePath=libextract-gstreamer.so

and the runtime does

  if (g_str_has_prefix(modulepath, "/")) {
    modulepath = g_build_filename(libdir, "tracker-0.16", "extract-modules", modulepath, NULL);
  }

The important part is to get the 'lib64' out of the rules files, since that will give you a lib<>lib64 difference on rh-style multilib systems

(In reply to comment #2)
> No, I would suggest that the rule file just contains
> 
> ModulePath=libextract-gstreamer.so
> 
> and the runtime does
> 
>   if (g_str_has_prefix(modulepath, "/")) {
>     modulepath = g_build_filename(libdir, "tracker-0.16", "extract-modules",
> modulepath, NULL);
>   }
> 
> The important part is to get the 'lib64' out of the rules files, since that
> will give you a lib<>lib64 difference on rh-style multilib systems

I see. That makes sense.

I do wonder why this would be any different in reality though. The ${libdir} which is passed at build time shouldn't change at runtime right? And libdir in the g_build_filename() example above is actually taken from build time variables. Forgive the persistence here, I just wonder how this can ever be different and where the problem really comes from :)

All of that aside, it does make sense to have less information in those rules files (regarding the path), I agree.

In a sense we do already have this feature by allowing the env var TRACKER_EXTRACTOR_RULES_DIR so it should be trivial to work this in.

(In reply to comment #3)
> I do wonder why this would be any different in reality though. The ${libdir}
> which is passed at build time shouldn't change at runtime right? And libdir in
> the g_build_filename() example above is actually taken from build time
> variables. Forgive the persistence here, I just wonder how this can ever be
> different and where the problem really comes from :)

Based on my limited understanding of multilib, I think it would matter if you were to have the tracker libraries installed for multiple architectures at the same time. RPM would see that the shared content in /usr/share/tracker is different for each architecture and freak out.

Created attachment 256912 [details] [review]
Add a newline at the end of file

Created attachment 256914 [details] [review]
libtracker-extract, tracker-extract: Remove $modulesdir from *.rules

Created attachment 256915 [details] [review]
Use $(MKDIR_P) instead of $(mkdir_p)

Comment on attachment 256914 [details] [review]
libtracker-extract, tracker-extract: Remove $modulesdir from *.rules

Superb patch thanks Rishi :)

The only thing I would add is, I wonder if it makes sense to have .in files at
all now. The sed call is also no longer useful AFAICS.

I might look to cleaning this up later on unless you fancy doing that too
Rishi?

Thanks again for the superb patches here :)

(In reply to comment #9)
> The only thing I would add is, I wonder if it makes sense to have .in files at
> all now. The sed call is also no longer useful AFAICS.

Yes, you are right. I will change them to *.rules.

Thanks for the review.

Created attachment 257007 [details] [review]
tracker-extract: Rename *.rules.in to *.rules

Comment on attachment 257007 [details] [review]
tracker-extract: Rename *.rules.in to *.rules

Looks good thanks Rishi