Re: [Tracker] Nemomobile patches

[Martyn Russell <martyn lanedo com>]

On 16/03/13 08:46, Philip Van Hoof wrote:
> Hi guys,
>
> Patches being applied to Tracker for Nemomobile:
>
> https://build.pub.meego.com/package/files?package=tracker&project=CE%
> 3AMW%3AShared
>
> Ideally we try to integrate as much as possible of these patches
> upstream.

So, I did some patch review:

- 0001-Remove-tracker-tests.aegis-from-config_SCRIPTS-if-ma.patch:
  I am actually wondering *why* this patch is needed.
  Is anyone using Aegis these days? It looks like we remove all Aegis
  reference if we're applying this sort of patch.

- 0002-Tracker-extract-Parse-the-video-filename-to-obtain-e.patch:
  I've seen this patch used in many projects but not actually applied
  to the Tracker code base. Perhaps we should have a configure option
  for it. It seems quite popular to have it. I tried applying it but
  realised it's not a simple thing to integrate nicely. IT would take a
  bit more time than I have right now. Mostly because we have this
  libtracker-extract guarantee API which I would prefer to integrate
  with. We already have a title sanity checking function and adding to
  that is what's needed. The patch that these guys are applying does
  absolutely nothing at all because the main function that does the
  work isn't ever called AFAICS. The good news is, it applies. The bad
  news is, you can't just call the function because it would add
  multiple SPARQL predicates for nie:title and a few others.

- 0005-Fix-missing-gobject-introspection-checks.patch:
  This shouldn't be needed any more. We have a --enable-introspection=no
  configure switch.

- tracker-0.10.37-fix-linking-with-newer-glib.patch:
  WE certainly don't use gthread linking, but we don't use any AFAICT
  so I wonder why this patch is needed. It's likely needed for earlier
  versions of libtracker-common, but not in master.

- tracker-0.10.37-fix-linking-with-newer-toolchain.patch:
  Irrelevant now we've removed the utils/playlists binary. This was
  likely just to fix the build, it has no impact on binaries installed
  to the system.

- tracker-forget-removed-files.patch:
  Already applied and in 0.15.4.

- tracker-miner-fs-deal-with-data-inserted-by-other-apps.patch:
  Already applied and in 0.15.4.

- tracker-tests-310-fts-indexing-use-graph-updated-signal.patch:
- tracker-tests-400-extractor-skip-unsupported-file-types.patch:
- tracker-tests-501-writeback-details-fix-invalid-metadata-key.patch:
- tracker-tests-allow-reuse-graph-updated-signal-handling.patch:
- tracker-tests-fix-helper-starting-order.patch:
- tracker-tests-fix-undefined-function-call.patch:
- tracker-tests-increase-timeout.patch:
- tracker-tests-make-400-extractor-work-with-testrunner.patch:
  Not had time to review and apply these. They're for the
  functional-tests and can be applied after release when I have more
  time. Many of these look right at a glance.

- tracker-tests-do-not-su-meego.patch:
  Applied.

- tracker-tests-synchronous-terminate-pre-step.patch:
  If the idea here is to kill, the functional-test should use -k not
  -t. I don't know if -t is used because the shutdown has to be clean
  or not. Using the while loop isn't clearly the right way to do this
  without more context.

Ivan, any chance you can review/test the functional-test cases? I know 
you had a hand in a lot of those.

Thanks,

--
Regards,
Martyn

Founder and CEO of Lanedo GmbH.