GNOME Bugzilla – Bug 329841
Month/Day directory without leading 0 (1-31 instead of 01-31)
Last modified: 2007-01-09 13:26:00 UTC
Please describe the problem: F-Spot stores all photos in ~/Photos/YYYY/MM/day/Photo Where YYYY = 2006 MM = 01 - 12 Day = 1 - 31 This causes the day directory to not be in a sorted order when you do a ls in the directory. bengt@jolly:~/Photos/2005/3$ ls 1 11 12 13 14 15 19 2 20 4 5 Steps to reproduce: 1. Import Photos 2. 3. Actual results: Day directory is created as 1 - 31 1 2 .. 31 Expected results: The day directory should be created like 01 - 31 01 02 .. 31 Does this happen every time? Yes Other information:
Created attachment 58678 [details] [review] Patch against CVS 060204 +// time.Month, +// time.Day); + time.ToString("MM"), // To get months with leading 0 as in 01 - 12 + time.ToString("dd")); // To get days with leading 0 as in 01 - 31 bengt@jolly:~/Photos$ ls */*/ 2005/02/: 27 2005/03/: 01 02 03 04 05 07 08 11 12 13 14 15 19 20 2006/01/: 15 17
I also found that by default the month is created as 1 - 12, which is the same problem as for the days. My above patch fixes both day and month.
The only problem with this patch is that if the user imports new photos from the same month/day as they already have, then there will be both a 1 and a 01 for instance. So really the fix is more complicated - we need to move the old directories and update that in the database.
Created attachment 59440 [details] [review] Patch agains CVS 2 lines changed for the actual import fix, and 311 lines or so to update existing data. try 1) Create Directory 2) hard link photo 3) update database 4) move thumbnails catch revert thumbnails remove new hard link remove new directory remove old hard link remove old directories Would appreciate some comments if anyone have any. Especially would appreciate if someone using a BSD system could verify this one. Do bear in mind that please test this on a test system, not on your live data. Also, just after 4 (in the above pseudo code) you can uncomment a raise exception so no changes will be made... Also uncomment the Readln comment, so you can check the file structure.
Forgot one small line in my pseudo code ;) after remove new directory... we raise the Exception again, so the database will be rolled back. But do not handle a power off though.
Comment from Ruben Patch probably do not cope with deleted pictures (a photo id, with missing photo!) Anyway, I missed testing this test case. Awkward with to many if statements, might be better using continue instead. Also, - throw new Exception (String.Format("Photo with name {0} do not exists", old_photo)); + throw new Exception (String.Format("Photo with name {0} does not exist", old_photo));
Created attachment 60749 [details] [review] Patch that handles deleted pictures I modified the patch to handle deleted pictures. In short - if (orig_directory.StartsWith (FSpot.Global.PhotoDirectory)) { + if ((orig_directory != null) && (orig_directory.StartsWith (FSpot.Global.PhotoDirectory))) { I have not changed from If/Then to use "continue" though. If that is a request I will look into it, or perhaps someone else could do that. I thought the code is nicely commented and easy to follow, but then again, I am biased :)
Related comment in bug #339078 ---- Anders Bo Rasmussen I don't think we should change existing filenames. Users might have other applications that remember the filenames and this will break if names of existing files change. ----
Been long time with no comments on this one. Is there a problem with getting this one into CVS or? If problem with the update function, just skip the update and let all new photos get the new filename.
Do we really need to update the existing directory structure? If so, should it really be done by using update? I mean, 300+ lines of code for this... Ok, not the best code perhaps but still.
Created attachment 73205 [details] [review] 2 liner patch for new imports This patch is an updated patch that applies cleanly against CVS. It only handles new imports, which means any old directory names are left as they are. So, you might have a 2006/9/1 and a 2006/09/01 directory after this patch. The update patch to ensure all old directory names were renamed is much more complicated, and perhaps not worth the effort.
I'm voting in favor of the 2-lines patch. Supporting and maintaining a 300+ LOC updater for only one use is probably too much efforts.
*** Bug 369201 has been marked as a duplicate of this bug. ***
Another version of a patch in bug #369201
Created attachment 75846 [details] [review] One-line patch to zero-pad month and day. This has the same function as Bengt's two-line patch, but does so by only adding 6 characters (using String.Format). It does not change the location of existing files-- it only affects where newly imported files will go. Brought here from bug #369201
fixed in r2680 (at last)