Moodle
  1. Moodle
  2. MDL-33812

backup_auto_keep not being honoured

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.1.6, 2.2.4, 2.3.1, 2.4
    • Fix Version/s: 2.2.5, 2.3.2
    • Component/s: Backup
    • Environment:
      Debian GNU/Linux (Squeeze), PostgreSQL 8.4.12, PHP 5.3.3
    • Database:
      PostgreSQL
    • Testing Instructions:
      Hide

      Test prerequisites

      (The followings are found in Home ► Site administration ► Courses ► Backups ► Automated backup setup)

      • Enable automated backups (backup_auto_active)
      • Set backup_auto_storage to 'Course backup filearea and the specified directory'
      • Set backup_auto_destination to some writeable path on your machine
      • Set backup_auto_keep to 2
      • Tick backup_shortname
      • Have at least two courses in your installation of Moodle

      Test 1

      1. Run `php admin/cli/automated_backups.php`
      2. Wait 2 minutes
      3. Run again `php admin/cli/automated_backups.php`
      4. Wait 2 minutes
      5. Run again again `php admin/cli/automated_backups.php`
      6. Wait 2 minutes
      7. Make sure you don't have more than two backups of each course in backup_auto_destination
      8. Go to Home ► Courses ► Champion ► Restore
      9. Make sure you don't have more than two backups under 'Automated backups'

      Test 2

      1. Set backup_shortname to false (untick)
      2. Keep any existing backup
      3. Repeat test 1
      Show
      Test prerequisites (The followings are found in Home ► Site administration ► Courses ► Backups ► Automated backup setup) Enable automated backups ( backup_auto_active ) Set backup_auto_storage to 'Course backup filearea and the specified directory' Set backup_auto_destination to some writeable path on your machine Set backup_auto_keep to 2 Tick backup_shortname Have at least two courses in your installation of Moodle Test 1 Run `php admin/cli/automated_backups.php` Wait 2 minutes Run again `php admin/cli/automated_backups.php` Wait 2 minutes Run again again `php admin/cli/automated_backups.php` Wait 2 minutes Make sure you don't have more than two backups of each course in backup_auto_destination Go to Home ► Courses ► Champion ► Restore Make sure you don't have more than two backups under 'Automated backups' Test 2 Set backup_shortname to false (untick) Keep any existing backup Repeat test 1
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-33812-master
    • Rank:
      41898

      Description

      backup_auto_keep has a default value of 1 (one), which I have not changed, yet there are currently SIX backups of each course in my backup_auto_destination directory.

      The value of backup_auto_keep also has no effect on the number of backups listed in the 'Automated Backups' section of the 'Restore' page for each course; only the most recent backup is ever listed at any one time.

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          I believe this is a duplicate of an issue that has just recently been fixed.

          Show
          Michael de Raadt added a comment - I believe this is a duplicate of an issue that has just recently been fixed.
          Hide
          Sebastian Tennant added a comment -

          I'm not sure if it is but I'm happy to wait until the next MOODLE_21_STABLE update to see if the problem has in fact been solved.

          Show
          Sebastian Tennant added a comment - I'm not sure if it is but I'm happy to wait until the next MOODLE_21_STABLE update to see if the problem has in fact been solved.
          Hide
          Sebastian Tennant added a comment -

          Please could you reopen this bug Michael. It was not a duplicate of MDL-33583. I've updated MOODLE_21_STABLE a few times since reporting this bug and I am still observing the same broken behaviour.

          Show
          Sebastian Tennant added a comment - Please could you reopen this bug Michael. It was not a duplicate of MDL-33583 . I've updated MOODLE_21_STABLE a few times since reporting this bug and I am still observing the same broken behaviour.
          Hide
          Sebastian Tennant added a comment -

          I repeat, please could you reopen this bug Michael. It was not a duplicate of MDL-33583. I've updated MOODLE_21_STABLE a few times since reporting this bug and I am still observing the same broken behaviour.

          Show
          Sebastian Tennant added a comment - I repeat, please could you reopen this bug Michael. It was not a duplicate of MDL-33583 . I've updated MOODLE_21_STABLE a few times since reporting this bug and I am still observing the same broken behaviour.
          Hide
          Michael de Raadt added a comment - - edited

          Hi, Petr.

          It looks like there is still a problem with the backup_auto_keep setting. Could you please look into this?

          Show
          Michael de Raadt added a comment - - edited Hi, Petr. It looks like there is still a problem with the backup_auto_keep setting. Could you please look into this?
          Hide
          Petr Škoda added a comment -

          I know nothing about backup, sorry.

          Show
          Petr Škoda added a comment - I know nothing about backup, sorry.
          Hide
          Petr Škoda added a comment -

          I suppose this backup file cleanup was originally implemented by Sam and later tweaked by Eloy, I just fixed a case when 0 was selected (meaning keep all) in the linked MDL-33583, I did not dive into details, sorry.

          Show
          Petr Škoda added a comment - I suppose this backup file cleanup was originally implemented by Sam and later tweaked by Eloy, I just fixed a case when 0 was selected (meaning keep all) in the linked MDL-33583 , I did not dive into details, sorry.
          Hide
          Petr Škoda added a comment -

          Eloy volunteered, thanks!

          Show
          Petr Škoda added a comment - Eloy volunteered, thanks!
          Hide
          Petr Škoda added a comment - - edited

          Hmm, maybe the problem is in the regex file name match:

          $filename = $backupword . '-' . backup::FORMAT_MOODLE . '-' . backup::TYPE_1COURSE . '-' .$course->id . '-';
          

          looks like the get_default_backup_filename() uses a different format with course shortname.

          Show
          Petr Škoda added a comment - - edited Hmm, maybe the problem is in the regex file name match: $filename = $backupword . '-' . backup::FORMAT_MOODLE . '-' . backup::TYPE_1COURSE . '-' .$course->id . '-'; looks like the get_default_backup_filename() uses a different format with course shortname.
          Hide
          Petr Škoda added a comment -

          rehmm, it does use id there, the problem must be elsewhere

          Show
          Petr Škoda added a comment - rehmm, it does use id there, the problem must be elsewhere
          Hide
          Sebastian Tennant added a comment -

          Just came across this forum post (from approx. 7 months ago):

          http://moodle.org/mod/forum/discuss.php?d=191275

          which confirms what I'm saying:

          "Apparently if you set a 'Specified directory for automated backups', the automatic removing doesn't work."

          since we have always had backup_auto_storage set to 'Specified directory for automated backups'.

          Show
          Sebastian Tennant added a comment - Just came across this forum post (from approx. 7 months ago): http://moodle.org/mod/forum/discuss.php?d=191275 which confirms what I'm saying: "Apparently if you set a 'Specified directory for automated backups', the automatic removing doesn't work." since we have always had backup_auto_storage set to 'Specified directory for automated backups'.
          Hide
          Michael de Raadt added a comment -

          Thanks for the additional details. I'm bumping this issue.

          Show
          Michael de Raadt added a comment - Thanks for the additional details. I'm bumping this issue.
          Hide
          Sebastian Tennant added a comment -

          We've just upgraded to MOODLE_23_STABLE and I can confirm that the problem persists. We upgraded two days ago and we already have two sets of course backups in our backup_auto_destination directory.

          Show
          Sebastian Tennant added a comment - We've just upgraded to MOODLE_23_STABLE and I can confirm that the problem persists. We upgraded two days ago and we already have two sets of course backups in our backup_auto_destination directory.
          Hide
          Frédéric Massart added a comment - - edited

          Hi everyone,

          I have found that this issue is caused by both the use of specific directory and backup_shortname. While checking for old backup files to delete, we match them using a regex which includes the course ID, not the course shortname. Whilst it could be fixed by handling the shortname in the regex, that could lead to potential data loss (it can contain '-' which is used in the file naming, can be updated, etc...).

          After a bit of discussion with Apu we came up with two ideas:

          • This can be fixed by forcing the ID to be in the file name no matter what. The backup_shortname setting should then be a little rephrased.
          • It would be better to handle the list of backup files in the database instead of directly working with the file names.

          Anyone's input is welcome!

          Show
          Frédéric Massart added a comment - - edited Hi everyone, I have found that this issue is caused by both the use of specific directory and backup_shortname . While checking for old backup files to delete, we match them using a regex which includes the course ID, not the course shortname. Whilst it could be fixed by handling the shortname in the regex, that could lead to potential data loss (it can contain '-' which is used in the file naming, can be updated, etc...). After a bit of discussion with Apu we came up with two ideas: This can be fixed by forcing the ID to be in the file name no matter what. The backup_shortname setting should then be a little rephrased. It would be better to handle the list of backup files in the database instead of directly working with the file names. Anyone's input is welcome!
          Hide
          Frédéric Massart added a comment -

          As the option to set the course name has been integrated only recently (MDL-28657), I thought that it wouldn't harm to tweak it a bit. As you can see, the ID will now always been present in the file name, which will not any current configuration.

          If we decide to handle the files differently in the future, that's probably better as part of another issue.

          Show
          Frédéric Massart added a comment - As the option to set the course name has been integrated only recently ( MDL-28657 ), I thought that it wouldn't harm to tweak it a bit. As you can see, the ID will now always been present in the file name, which will not any current configuration. If we decide to handle the files differently in the future, that's probably better as part of another issue.
          Hide
          Martin Dougiamas added a comment -

          +1 for the first one. Backup files may come and go, admins may manage them from shell, for example.

          Show
          Martin Dougiamas added a comment - +1 for the first one. Backup files may come and go, admins may manage them from shell, for example.
          Hide
          Rajesh Taneja added a comment -

          Patch looks good Fred, although you might consider not to change variable name, as it is not required.

          Also, I am not sure about any need for changing string https://github.com/FMCorz/moodle/compare/MDL-33812-master#L1L71, it will be nice to keep original value.

          Show
          Rajesh Taneja added a comment - Patch looks good Fred, although you might consider not to change variable name, as it is not required. Also, I am not sure about any need for changing string https://github.com/FMCorz/moodle/compare/MDL-33812-master#L1L71 , it will be nice to keep original value.
          Hide
          Frédéric Massart added a comment -

          Thank you for your review Raj. After discussing with a well-known integrator, renaming the variable does not look like an issue. Regarding the language string, you were right, that could be tricky. But as the meaning does not change that much, that would be all right for anterior versions to have it.

          Pushing for integration. \o/

          Show
          Frédéric Massart added a comment - Thank you for your review Raj. After discussing with a well-known integrator, renaming the variable does not look like an issue. Regarding the language string, you were right, that could be tricky. But as the meaning does not change that much, that would be all right for anterior versions to have it. Pushing for integration. \o/
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Oh, it seems we are suffering some sort of overlap/duplication with different issues here.

          • This is a clear regression caused by MDL-28657. So I'd say this needs to go to all the branches where that issue was implemented.
          • I've closed as duplicate of this: MDL-34146
          • Last week this landed: MDL-33531. Please analyze if that fixed this, or if there are code introduced there that needs to be reverted if this lands.

          Also note that such a change in backup filenames requires to be documented in release notes, because it's possible that some scripts out there are relying in current (id OR shortname) naming schema. So once this lands, we need to comment about it in all release notes / upgrade.txt files. So I'm adding the corresponding docs flag here.

          So I'm reopening, because I think that we need to decide if MDL-33531 is enough or we want the solution here too (id + shortname). I think this patch is perfect, but needs reconsideration based on last's week one.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Oh, it seems we are suffering some sort of overlap/duplication with different issues here. This is a clear regression caused by MDL-28657 . So I'd say this needs to go to all the branches where that issue was implemented. I've closed as duplicate of this: MDL-34146 Last week this landed: MDL-33531 . Please analyze if that fixed this, or if there are code introduced there that needs to be reverted if this lands. Also note that such a change in backup filenames requires to be documented in release notes, because it's possible that some scripts out there are relying in current (id OR shortname) naming schema. So once this lands, we need to comment about it in all release notes / upgrade.txt files. So I'm adding the corresponding docs flag here. So I'm reopening, because I think that we need to decide if MDL-33531 is enough or we want the solution here too (id + shortname). I think this patch is perfect, but needs reconsideration based on last's week one. Ciao
          Hide
          CiBoT added a comment -

          Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

          Show
          CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
          Hide
          Frédéric Massart added a comment -

          Hi Eloy,

          I was not aware of MDL-33531. It fixes the issue but leave place for another bug to come later on. I don't think it is safe to rely on the shortname while deleting the files. In MDL-33531, the deletion process looks for files names "backup-moodle2-course-this_is_my_course-[...].mbz", but what if someone names its course 1234, and that 1234 is the ID of another course, or that another course is named 1234-5678. Then those files will be considered as part of the same course and will be deleted. It could lead to some strange behaviour if the course name is updated as well.

          The solution I provided here forced the ID to be in the file name so that we don't have to change the deletion mechanism. Whether we decide to integrate this issue or not, both cannot live to together. Because one would force the ID in the file name, and the other would still try to delete the backups using the shortname.

          (Added David as a watcher, as he worked on MDL-33531)

          Show
          Frédéric Massart added a comment - Hi Eloy, I was not aware of MDL-33531 . It fixes the issue but leave place for another bug to come later on. I don't think it is safe to rely on the shortname while deleting the files. In MDL-33531 , the deletion process looks for files names "backup-moodle2-course-this_is_my_course- [...] .mbz", but what if someone names its course 1234, and that 1234 is the ID of another course, or that another course is named 1234-5678. Then those files will be considered as part of the same course and will be deleted. It could lead to some strange behaviour if the course name is updated as well. The solution I provided here forced the ID to be in the file name so that we don't have to change the deletion mechanism. Whether we decide to integrate this issue or not, both cannot live to together. Because one would force the ID in the file name, and the other would still try to delete the backups using the shortname. (Added David as a watcher, as he worked on MDL-33531 )
          Hide
          Frédéric Massart added a comment - - edited

          I am pushing this for integration again. I'll leave it up to the integrators to decide what to do. In case this is integrated, I have reverted MDL-33531 in my branches so that it does not have to be done during integration.

          Edited: fixing one link, lol, it was driving me crazy.

          Show
          Frédéric Massart added a comment - - edited I am pushing this for integration again. I'll leave it up to the integrators to decide what to do. In case this is integrated, I have reverted MDL-33531 in my branches so that it does not have to be done during integration. Edited: fixing one link, lol, it was driving me crazy.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          I agree, that having always the id there really helps for better knowing, without any conflict, the source course.

          But there is one problem, what happens with all the sites having the backup_shortname option enabled right now. Those backups won't be deleted ever, because they don't match anymore, so they will get 2x the number of copies forever.

          So, at this point, there are 2 possibilities:

          1) Make this change only for master and provide some tool to cleanup old backups using the old matching algorithm, documenting it in the release and upgrade notes.

          2) Make this change in master and stables, but keeping the old matching algorithm as fallback if no new matches are found (so in X iterations, all sites will be clean of old backups). master will use only the new matching way. Of course, this needs also documentation, because people relying in that matching schema would need to adjust stuff.

          So.. I'm going to reopen this for now. Once again, code is perfect, but we need to define the patch to follow with those "old" backups.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - I agree, that having always the id there really helps for better knowing, without any conflict, the source course. But there is one problem, what happens with all the sites having the backup_shortname option enabled right now. Those backups won't be deleted ever, because they don't match anymore, so they will get 2x the number of copies forever. So, at this point, there are 2 possibilities: 1) Make this change only for master and provide some tool to cleanup old backups using the old matching algorithm, documenting it in the release and upgrade notes. 2) Make this change in master and stables, but keeping the old matching algorithm as fallback if no new matches are found (so in X iterations, all sites will be clean of old backups). master will use only the new matching way. Of course, this needs also documentation, because people relying in that matching schema would need to adjust stuff. So.. I'm going to reopen this for now. Once again, code is perfect, but we need to define the patch to follow with those "old" backups. Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Please, discuss and agree any solution @ HQ. I'm happy with any alternative.

          Show
          Eloy Lafuente (stronk7) added a comment - Please, discuss and agree any solution @ HQ. I'm happy with any alternative.
          Hide
          CiBoT added a comment -

          Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

          Show
          CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
          Hide
          Frédéric Massart added a comment -

          I personally think that this should be integrated, here are my thoughts:

          1. The patch integrated last week can lead to losing backups when course short names have the same prefix. If we don't integrate this issue, IMO we should revert MDL-33531. IE: Course 1 = 'myname', Course 2 = 'myname-mylastname'. Backup files for Course 2 will be considered as part of Course 1.
          2. So far, none of the backups including short names were deleted. Integrating this would just leave the existing backups there, it is not a regression.
          3. Deleting the old short name backups is a tricky task. It could surely be done by reading the zip file (Eloy's idea during a chat), but it implies a smart logic to properly count the backups with short names, and the backups with IDs and deleting only the right ones. IE: If we have 60 short name backups and 2 with IDs, but the admin wants to keep the 30 oldest backup, then the logic to delete the old backup files using short names would have to keep 28 of them.
          4. Backporting should not be a problem as there is not real API change, and the string changed keeps an accurate meaning.
          Show
          Frédéric Massart added a comment - I personally think that this should be integrated, here are my thoughts: The patch integrated last week can lead to losing backups when course short names have the same prefix. If we don't integrate this issue, IMO we should revert MDL-33531 . IE: Course 1 = 'myname', Course 2 = 'myname-mylastname'. Backup files for Course 2 will be considered as part of Course 1. So far, none of the backups including short names were deleted. Integrating this would just leave the existing backups there, it is not a regression. Deleting the old short name backups is a tricky task. It could surely be done by reading the zip file (Eloy's idea during a chat), but it implies a smart logic to properly count the backups with short names, and the backups with IDs and deleting only the right ones. IE: If we have 60 short name backups and 2 with IDs, but the admin wants to keep the 30 oldest backup, then the logic to delete the old backup files using short names would have to keep 28 of them. Backporting should not be a problem as there is not real API change, and the string changed keeps an accurate meaning.
          Hide
          David Monllaó added a comment -

          Hi,

          • If we can just "leave there" the backups with the old shortname format this patch is ok and my +1 to revert MDL-33531; to delete the backups with the old shortname format is hard and requires to unzip the file + read an XML, with a lot of courses or backups it could be too costly
          • If we should delete the old backups I think that could be easier to keep MDL-33531 but adding a str_replace('', '', $shortname) this way the upgrade process only needs to rename the files (replacing the shortname '' for '') without having to unzip + open XML every backup; it seems an affordable cost
          Show
          David Monllaó added a comment - Hi, If we can just "leave there" the backups with the old shortname format this patch is ok and my +1 to revert MDL-33531 ; to delete the backups with the old shortname format is hard and requires to unzip the file + read an XML, with a lot of courses or backups it could be too costly If we should delete the old backups I think that could be easier to keep MDL-33531 but adding a str_replace(' ', ' ', $shortname) this way the upgrade process only needs to rename the files (replacing the shortname ' ' for ' ') without having to unzip + open XML every backup; it seems an affordable cost
          Hide
          Eloy Lafuente (stronk7) added a comment -

          As commented by chat, I agree that always prefixing with id is the winner, 100%. And the sooner we switch the better.

          But some things must be addressed before/together/after this lands:

          • Documentation of the changes in the the backup file name schema. This must go, for sure, to release notes, and perhaps also to upgrade.txt, under the corresponding version notes and specifying: "Since 2.x.x, automated backup filenames are...". People with all sort of automated tasks used to backup/move those files need to know about the change.
          • The change use of negative vars/params, aka: $withoutname. My +1 to change this since day 0 too and spread the change to Docs and upgrade.txt files. I don't think this is being used too much out from moodle core, but it's public and, as such, must be documented.
          • The "orphaned" backups case (that won't match for deletion anymore). We need to have at least that problem explained in one new issue, to avoid forgetting as a minimum and to provide one solution as ideal goal). I really think that the change proposed here can have some important impact in sites out there (doubling and never deleting the number of backups per course).

          So with those 3 points, I'm all about to perform the change ASAP. Once again, I agree that the only practical solution is to always use the id in the names.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - As commented by chat, I agree that always prefixing with id is the winner, 100%. And the sooner we switch the better. But some things must be addressed before/together/after this lands: Documentation of the changes in the the backup file name schema. This must go, for sure, to release notes, and perhaps also to upgrade.txt, under the corresponding version notes and specifying: "Since 2.x.x, automated backup filenames are...". People with all sort of automated tasks used to backup/move those files need to know about the change. The change use of negative vars/params, aka: $withoutname. My +1 to change this since day 0 too and spread the change to Docs and upgrade.txt files. I don't think this is being used too much out from moodle core, but it's public and, as such, must be documented. The "orphaned" backups case (that won't match for deletion anymore). We need to have at least that problem explained in one new issue, to avoid forgetting as a minimum and to provide one solution as ideal goal). I really think that the change proposed here can have some important impact in sites out there (doubling and never deleting the number of backups per course). So with those 3 points, I'm all about to perform the change ASAP. Once again, I agree that the only practical solution is to always use the id in the names. Ciao
          Hide
          Frédéric Massart added a comment -

          Hi Eloy,

          I have added am upgrade.txt file in backup, not sure it's the best place to put it though.
          I don't like negative variables either, I don't know why I used them in the first place, it's now renamed.
          I have created a new issue to take care of the extra files in MDL-35116.

          After several discussion about this here and there, I think we should change the way we sort files to delete them. I have created MDL-35117 to address this problem. The idea of reading the XML info from the backup file is brilliant, and also relying on the modification date is scary as some OS have strange behaviours about that.

          Thanks!

          Show
          Frédéric Massart added a comment - Hi Eloy, I have added am upgrade.txt file in backup, not sure it's the best place to put it though. I don't like negative variables either, I don't know why I used them in the first place, it's now renamed. I have created a new issue to take care of the extra files in MDL-35116 . After several discussion about this here and there, I think we should change the way we sort files to delete them. I have created MDL-35117 to address this problem. The idea of reading the XML info from the backup file is brilliant, and also relying on the modification date is scary as some OS have strange behaviours about that. Thanks!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated (22, 23 & master), thanks!

          Note: I was debating myself about the string change in stable branches. We don't use to change the "meaning" of strings there, because people not upgrading (using, for example 2.2.0) will get the new strings when behavior continues being the old one. But finally, in this case, I decided the change was minor and the result not contradictory. Just remember that in stables, it's always preferred to create a new string.

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated (22, 23 & master), thanks! Note: I was debating myself about the string change in stable branches. We don't use to change the "meaning" of strings there, because people not upgrading (using, for example 2.2.0) will get the new strings when behavior continues being the old one. But finally, in this case, I decided the change was minor and the result not contradictory. Just remember that in stables, it's always preferred to create a new string.
          Hide
          Adrian Greeve added a comment -

          Tested on 2.2, 2.3 and master.
          Only two backups were kept at any one stage (with the correct configuration).
          No problems encountered.
          Test passed.

          Show
          Adrian Greeve added a comment - Tested on 2.2, 2.3 and master. Only two backups were kept at any one stage (with the correct configuration). No problems encountered. Test passed.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Many thanks for the hard work.

          These changes have been spread upstream and are already available in the git and cvs repositories.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Many thanks for the hard work. These changes have been spread upstream and are already available in the git and cvs repositories. Ciao
          Hide
          Mary Cooch added a comment -

          (Housekeeping) Removing docs_required as I don't think user docs are needed- please say if you disagree.

          Show
          Mary Cooch added a comment - (Housekeeping) Removing docs_required as I don't think user docs are needed- please say if you disagree.

            People

            • Votes:
              3 Vote for this issue
              Watchers:
              9 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: