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 733536 - Remove --enable-guarantee metadata, always guarantee nie:title and nie:contentCreated
Remove --enable-guarantee metadata, always guarantee nie:title and nie:conten...
Status: RESOLVED FIXED
Product: tracker
Classification: Core
Component: General
git master
Other Linux
: Normal minor
: ---
Assigned To: tracker-general
Depends on:
Blocks:
 
 
Reported: 2014-07-22 09:17 UTC by Sam Thursfield
Modified: 2014-12-03 11:28 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Always guarantee metadata, remove --enable-guarantee-metadata option (6.53 KB, patch)
2014-07-22 09:19 UTC, Sam Thursfield
reviewed Details | Review
Revert "Always guarantee metadata, remove --enable-guarantee-metadata option" (5.71 KB, patch)
2014-11-20 15:07 UTC, Debarshi Ray
none Details | Review
Revert "Always guarantee metadata, remove --enable-guarantee-metadata option" (5.71 KB, patch)
2014-11-28 16:52 UTC, Debarshi Ray
committed Details | Review

Description Sam Thursfield 2014-07-22 09:17:25 UTC
The --enable-guarantee-metadata flag ensures that nie:title and nie:contentCreated will always be set for a given file, even if they need to be guessed based on the filename and mtime. It was disabled by default, although the functional tests rely on it being enabled.

I see no harm in making this behaviour the default, and removing the option. This simplifies the code and removes one way that the tests can break.
Comment 1 Sam Thursfield 2014-07-22 09:19:20 UTC
Created attachment 281368 [details] [review]
Always guarantee metadata, remove --enable-guarantee-metadata option

The --enable-guarantee-metadata flag ensures that nie:title and
nie:contentCreated will always be set for a given file, even if they
need to be guessed based on the filename and mtime. It was disabled
by default, although the functional tests rely on it being enabled.

I see no harm in making this behaviour the default, and removing the
option. This simplifies the code and removes one way that the tests can
break.
Comment 2 Martyn Russell 2014-07-22 10:29:38 UTC
Comment on attachment 281368 [details] [review]
Always guarantee metadata, remove --enable-guarantee-metadata option

I propose a (slightly) different approach.

Why not have this on by default and make it possible to disable this feature.
We could remove all of the #ifdefs in place except where we actually call the API tracker_guarantee_*().

This would make the maintenance much easier, improve metadata in places and allows those not happy with this "guessing" to disable it when they want.

My main concern here, is that I wanted to enable this before like you Sam, but there was opposition to it back then and I would imagine there still is.

This is also quite similar to the bug:

https://bugzilla.gnome.org/show_bug.cgi?id=607548

Perhaps we should incorporate that work into this/these patch/patches?

Thoughts?
Comment 3 Sam Thursfield 2014-07-22 15:25:02 UTC
It's a fair point made in #607548 about renames not being reflected in nie:title. Although for nie:contentCreated there should be no issue.

I don't think having this conditional is a good idea, really. Tracker should just get things right.

As a point of reference, the Fedora package doesn't enable guaranteed metadata at all, so presumably GNOME works fine without that flag: http://pkgs.fedoraproject.org/cgit/tracker.git/tree/tracker.spec#n168

My only interest in enabling it was to make the functional-tests slightly less broken by default :) I'd be happy to make the functional-tests not depend on guaranteed metadata, instead.
Comment 4 Martyn Russell 2014-07-23 09:34:43 UTC
OK, you've convinced me, we want maintenance to be easier anyway - people can always reopen this bug or complain ;)

I've not reviewed your patch in detail, but you're pretty much a maintainer anyway, please go ahead Sam.

Thanks again! :)
Comment 5 Sam Thursfield 2014-07-24 16:56:36 UTC
OK, so to be clear, we'll force-enable this feature, but if it turns out that the changed behaviour of nie:title is undesirable, we'll either remove it or fix it so that it doesn't suck.

Sounds good to me, I'll do that.
Comment 6 Sam Thursfield 2014-07-26 14:06:18 UTC
commit 898ac3ca17afa5d9fb382656e7e4ba4ff4b6ef39
Author: Sam Thursfield <sam@afuera.me.uk>
Date:   Mon Jul 21 21:57:17 2014 +0100

    Always guarantee metadata, remove --enable-guarantee-metadata option
    
    The --enable-guarantee-metadata flag ensures that nie:title and
    nie:contentCreated will always be set for a given file, even if they
    need to be guessed based on the filename and mtime.
    
    It was previously disabled by default, although the functional tests
    rely on it being enabled.
    
    There should be no harm in making this behaviour the default, and
    removing the option. If this turns out to have unintended consequences,
    we should either fix the code in question, or remove it completely.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=733536
Comment 7 Debarshi Ray 2014-11-07 09:51:31 UTC
(In reply to comment #3)

Sorry for not reacting to this earlier.

> As a point of reference, the Fedora package doesn't enable guaranteed metadata
> at all, so presumably GNOME works fine without that flag:
> http://pkgs.fedoraproject.org/cgit/tracker.git/tree/tracker.spec#n168

Although, I don't think it was a conscious decision to turn it off, it was actually helpful. We deliberately hide the file name in some cases. eg., in the grid view in gnome-photos where the basename is often IMG00042 or something human unreadable like that. And in other cases we when the nie:title is unset we fallback to the nfo:fileName. eg., in Documents where the chances of the file name being machine generated is lower. So having the nie:title clearly separate from the nfo:fileName was, kind of, useful in this case.

We can add some logic to the applications to check if nie:title is a suffix of the nfo:fileName, but that can break when files are renamed.
Comment 8 Debarshi Ray 2014-11-20 15:07:04 UTC
Created attachment 291103 [details] [review]
Revert "Always guarantee metadata, remove --enable-guarantee-metadata option"

After some discussion in #tracker on GIMPNet we decided to revert the previous patch.

There were some conflicts due to 81609a05b41d2b04d183fe7ec3ef1aae39ebf645 and the tracker-guarantee tests for dates don't work when the flag is disabled. It is due to the builder being in an invalid state when insert_close is called.

The libmediaart changes since this patch was merged makes it a bit complicated to go back in time and check if they worked before. So, if someone has a clue while I dig into it ...
Comment 9 Debarshi Ray 2014-11-28 16:52:55 UTC
Created attachment 291731 [details] [review]
Revert "Always guarantee metadata, remove --enable-guarantee-metadata option"

Fix the bug URL
Comment 10 Martyn Russell 2014-12-03 11:26:16 UTC
Comment on attachment 291731 [details] [review]
Revert "Always guarantee metadata, remove --enable-guarantee-metadata option"

Thanks for the revert, I've fixed the unit tests too.
Comment 11 Martyn Russell 2014-12-03 11:28:11 UTC
This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.