Moodle
  1. Moodle
  2. MDL-29474

A way for plugins to declare that they depend on other plugins

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1.1
    • Fix Version/s: 2.2
    • Component/s: Installation
    • Labels:
    • Testing Instructions:
      Hide

      For all these tests, repeat with CLI upgrade and upgrade through the web-browser.

      1. Upgrade your Moodle. Make sure the dependency info appears on the plugin-check page, and that then the upgrade works.

      2. Test a clean install to ensure there are no regressions.

      3. Test an upgrade from a Moodle 2.1 install to make sure there are no regressions.

      4. Check that admin/plugins.php and admin/environment.php pages still work.

      5. Edit some version.php files to test upgrades with problems. For example,
      a. try making mod/forum require a non-existant filter_frog, or something
      b. or make a plugin depend on a Moodle version or another plugin version in the future.
      Make sure that the plugin-check page shows the problem and refuses to let you upgrade.

      Show
      For all these tests, repeat with CLI upgrade and upgrade through the web-browser. 1. Upgrade your Moodle. Make sure the dependency info appears on the plugin-check page, and that then the upgrade works. 2. Test a clean install to ensure there are no regressions. 3. Test an upgrade from a Moodle 2.1 install to make sure there are no regressions. 4. Check that admin/plugins.php and admin/environment.php pages still work. 5. Edit some version.php files to test upgrades with problems. For example, a. try making mod/forum require a non-existant filter_frog, or something b. or make a plugin depend on a Moodle version or another plugin version in the future. Make sure that the plugin-check page shows the problem and refuses to let you upgrade.
    • Affected Branches:
      MOODLE_21_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:
    • Rank:
      19041

      Description

      The plan is to add a new option in version.php:

      $plugin->dependencies = array(
          'mod_quiz' => 2011090100,
          'qtype_pmatch' => ANY_VERSION,
      );
      

        Issue Links

          Activity

          Hide
          Tim Hunt added a comment -

          I'm taking this because some of our question types need it.

          Show
          Tim Hunt added a comment - I'm taking this because some of our question types need it.
          Hide
          Marina Glancy added a comment - - edited

          I would suggest:

          $plugin->dependencies[] = 'mod_quiz:2011090100';
          $plugin->dependencies[] = 'qtype_pmatch';
          

          (easier to read and to parse in my archive validator)

          Show
          Marina Glancy added a comment - - edited I would suggest: $plugin->dependencies[] = 'mod_quiz:2011090100'; $plugin->dependencies[] = 'qtype_pmatch'; (easier to read and to parse in my archive validator)
          Hide
          Tim Hunt added a comment -

          No. You need to make your parse better. Something like

          $dependancies = array();
          if (preg_match('~\$plugin->dependancies\s*=\s*array\s*\(([^;])\);~', $versionphp, $matches)) {
              foreach (explode(',', $matches[1]) as $bit) {
                  list($plugin, $version) = explode('=>', $bit, 2);
                  $plugin = clean_param($plugin, PARAM_ALPHANUMEXT);
                  $version = trim($version);
                  if ($version === 'ANY_VERSION') {
                      $version = 0;
                  }
                  $dependancies[$plugin] = clean_param($version, PARAM_FLOAT);
              }    
          }
          
          Show
          Tim Hunt added a comment - No. You need to make your parse better. Something like $dependancies = array(); if (preg_match('~\$plugin->dependancies\s*=\s*array\s*\(([^;])\);~', $versionphp, $matches)) { foreach (explode(',', $matches[1]) as $bit) { list($plugin, $version) = explode('=>', $bit, 2); $plugin = clean_param($plugin, PARAM_ALPHANUMEXT); $version = trim($version); if ($version === 'ANY_VERSION') { $version = 0; } $dependancies[$plugin] = clean_param($version, PARAM_FLOAT); } }
          Hide
          Tim Hunt added a comment -

          Informal list of sub-tasks,

          1. Extend plugin_manager in lib/pluginlib.php to load these new values, make them available to new code, and check the dependencies.
          2. Extend core_admin_renderer::plugins_check to display the dependency information.
          3. Change admin/qtypes.php and admin/qbehaviours.php to use plugin_manager and these dependencies, rather than it's existing custom code.
          4. Strip out the old ->requires_qtypes() and similar methods in the question code.
          Show
          Tim Hunt added a comment - Informal list of sub-tasks, Extend plugin_manager in lib/pluginlib.php to load these new values, make them available to new code, and check the dependencies. Extend core_admin_renderer::plugins_check to display the dependency information. Change admin/qtypes.php and admin/qbehaviours.php to use plugin_manager and these dependencies, rather than it's existing custom code. Strip out the old ->requires_qtypes() and similar methods in the question code.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Just for reference a slightly different code to parse dependencies, sure you'll pick ideas from both alternatives: http://pastebin.com/dqXtAXwQ

          Show
          Eloy Lafuente (stronk7) added a comment - Just for reference a slightly different code to parse dependencies, sure you'll pick ideas from both alternatives: http://pastebin.com/dqXtAXwQ
          Hide
          Marina Glancy added a comment -

          there are a lot of ways to parse. In this case I would probably use tokenizer instead of regexps (I already use it to strip out comments). This is definitely not the major problem here. Validator will parse whatever we decide.

          I just think that in the most cases people will not need to specify versions of plugin they depend on.

          Show
          Marina Glancy added a comment - there are a lot of ways to parse. In this case I would probably use tokenizer instead of regexps (I already use it to strip out comments). This is definitely not the major problem here. Validator will parse whatever we decide. I just think that in the most cases people will not need to specify versions of plugin they depend on.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Yeah, agree that in "most" cases that won't be necessary, but there are some plugins that, indeed, will need that (if the plugin requires something introduced by the parent's API at a given moment).

          So +1 goes to the ANY_VERSION / REAL_VERSION as specified originally by Tim (perhaps we could, in fact, omit the ANY_VERSION bit. But IMO it's clearer if specified, and surely easier to parse).

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Yeah, agree that in "most" cases that won't be necessary, but there are some plugins that, indeed, will need that (if the plugin requires something introduced by the parent's API at a given moment). So +1 goes to the ANY_VERSION / REAL_VERSION as specified originally by Tim (perhaps we could, in fact, omit the ANY_VERSION bit. But IMO it's clearer if specified, and surely easier to parse). Ciao
          Hide
          Tim Hunt added a comment -

          Right. This should be ready to go.

          The 'other require plugins' information is just displayed on the plugin_check screen, not on the plugins_control_panel (which is already quite big).

          I hope the fact that I could not resist improving the existing code is not a problem.

          Show
          Tim Hunt added a comment - Right. This should be ready to go. The 'other require plugins' information is just displayed on the plugin_check screen, not on the plugins_control_panel (which is already quite big). I hope the fact that I could not resist improving the existing code is not a problem.
          Hide
          David Mudrak added a comment -

          Thanks Tim for improving the pluginlib code while working on this. Here are some tiny details I spotted while reading the patch.

          • get_plugin_info() docblock typo: informaiton
          • get_plugin_info() could declare "@return plugin_information|null ..."
          • You may wish to drop the EOF change in mod/forum/version.php
          Show
          David Mudrak added a comment - Thanks Tim for improving the pluginlib code while working on this. Here are some tiny details I spotted while reading the patch. get_plugin_info() docblock typo: informaiton get_plugin_info() could declare "@return plugin_information|null ..." You may wish to drop the EOF change in mod/forum/version.php
          Hide
          Tim Hunt added a comment -

          Thanks David, I will fix that up tomorrow and then submit for integration.

          Show
          Tim Hunt added a comment - Thanks David, I will fix that up tomorrow and then submit for integration.
          Hide
          Tim Hunt added a comment -

          Last too commits amended to fix the problems David found.

          I'm submitting this for integration now, but if anyone else wants to look at it before next week, I am happy to have more comments. Thanks.

          Show
          Tim Hunt added a comment - Last too commits amended to fix the problems David found. I'm submitting this for integration now, but if anyone else wants to look at it before next week, I am happy to have more comments. Thanks.
          Hide
          Tim Hunt added a comment -

          OK, after a lot more refactoring (which I think has made the upgrade/install code much nicer) this is now ready for peer-review again.

          In fact, all that last bit of refactoring was just so that it was easy to hide the continue button on the plugin check page if there were unsatisfied requirements!

          Show
          Tim Hunt added a comment - OK, after a lot more refactoring (which I think has made the upgrade/install code much nicer) this is now ready for peer-review again. In fact, all that last bit of refactoring was just so that it was easy to hide the continue button on the plugin check page if there were unsatisfied requirements!
          Hide
          Tim Hunt added a comment -

          Note that I have tested New install and upgrade from 2.1 thorough the browser UI. Please can someone test the CLI interface.

          Also, dear integrator, please note that the branch contains separate commits for things that are just code clean-up, and things that are real changes.

          Show
          Tim Hunt added a comment - Note that I have tested New install and upgrade from 2.1 thorough the browser UI. Please can someone test the CLI interface. Also, dear integrator, please note that the branch contains separate commits for things that are just code clean-up, and things that are real changes.
          Hide
          Tim Hunt added a comment -

          Submitting for integration.

          (If anyone objects to that, feel free to reset it back to peer review.)

          Show
          Tim Hunt added a comment - Submitting for integration. (If anyone objects to that, feel free to reset it back to peer review.)
          Hide
          Eloy Lafuente (stronk7) added a comment -

          ... or how a simple idea can become a monster...

          great job! Some comments:

          1) not sure if, when one plugin is not satisfying dependencies, it's correct to show one green "To be installed" in the status column, perhaps same sort or red with "problem" or "dependencies unmeet" or so would be better.

          2) not sure if having those "legacy dependencies" is a good idea. At least they are a fallback way only, correct? They are not evaluated if proper (version.php) dependencies are found.

          3) what happens if you delete one "dependon" (aka parent) plugin? The dependency won't be satisfied anymore and the "dependent" plugin will be, somehow, orphaned (and surely non working/breaking) if the dependency was hard. Uhm, that's about to transverse all the plugins checking if the candidate has dependent plugins before proceeding with deletion.

          4) the environment page continues using smaller sizes than the rest of admin/install pages. Not important but, given you've moved it to admin_renderer and bit of CSS could be ok there.

          5) Should the dependencies information be shown also in the plugins overview page and not only @ install/upgrade?

          And that is, the earlier this lands, the better... awaiting for some feedback before integration. Nice hack.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - ... or how a simple idea can become a monster... great job! Some comments: 1) not sure if, when one plugin is not satisfying dependencies, it's correct to show one green "To be installed" in the status column, perhaps same sort or red with "problem" or "dependencies unmeet" or so would be better. 2) not sure if having those "legacy dependencies" is a good idea. At least they are a fallback way only, correct? They are not evaluated if proper (version.php) dependencies are found. 3) what happens if you delete one "dependon" (aka parent) plugin? The dependency won't be satisfied anymore and the "dependent" plugin will be, somehow, orphaned (and surely non working/breaking) if the dependency was hard. Uhm, that's about to transverse all the plugins checking if the candidate has dependent plugins before proceeding with deletion. 4) the environment page continues using smaller sizes than the rest of admin/install pages. Not important but, given you've moved it to admin_renderer and bit of CSS could be ok there. 5) Should the dependencies information be shown also in the plugins overview page and not only @ install/upgrade? And that is, the earlier this lands, the better... awaiting for some feedback before integration. Nice hack. Ciao
          Hide
          Tim Hunt added a comment -

          1) I am not sure. The code for the new plugin has been added to Moodle, and so it will be installed, once the dependencies are satisfied. So, I think it is OK for the status to show what it is we are trying to do, independent of the dependency status.

          2) There is a separate bug report (MDL-29808) to fix that legacy stuff after this thing has been integrated. I thought these changes were big enough already. They are only a fall-back.

          3) Yes, we should check dependancies before displaying the delete link. Can I suggest that we do that later? Please create a new MDL (you can assign it to me).

          4) Why don't you just do any CSS tweaks you like as part of integrating this. I did not think about that. I just moved the existing HTML/CSS to the renderer/theme.

          5) I was not sure about this. The plugins overview page is already very big and complex, so I thought it would not fit. However, if we are going to start hiding the delete button for required plugins, perhaps we should change it to show the reverse dependancy: "Cannot delete, required by ...." in that table cell. In which case, the can be part of the same future MDL as 3.

          Show
          Tim Hunt added a comment - 1) I am not sure. The code for the new plugin has been added to Moodle, and so it will be installed, once the dependencies are satisfied. So, I think it is OK for the status to show what it is we are trying to do, independent of the dependency status. 2) There is a separate bug report ( MDL-29808 ) to fix that legacy stuff after this thing has been integrated. I thought these changes were big enough already. They are only a fall-back. 3) Yes, we should check dependancies before displaying the delete link. Can I suggest that we do that later? Please create a new MDL (you can assign it to me). 4) Why don't you just do any CSS tweaks you like as part of integrating this. I did not think about that. I just moved the existing HTML/CSS to the renderer/theme. 5) I was not sure about this. The plugins overview page is already very big and complex, so I thought it would not fit. However, if we are going to start hiding the delete button for required plugins, perhaps we should change it to show the reverse dependancy: "Cannot delete, required by ...." in that table cell. In which case, the can be part of the same future MDL as 3.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated, thanks!

          Creating followup for 3 & 5 above and trying to improve CSS of environment table a bit...

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated, thanks! Creating followup for 3 & 5 above and trying to improve CSS of environment table a bit...
          Hide
          Eloy Lafuente (stronk7) added a comment -

          This surely requires documentation somewhere, starting from release changes pointing to it.

          Show
          Eloy Lafuente (stronk7) added a comment - This surely requires documentation somewhere, starting from release changes pointing to it.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Linked with:

          • MDL-29945 (plugin deletion checks, points 3 & 5 above)
          • MDL-29100 (improve environmental page CSS).

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Linked with: MDL-29945 (plugin deletion checks, points 3 & 5 above) MDL-29100 (improve environmental page CSS). Ciao
          Hide
          Sam Hemelryk added a comment -

          Ficken awesome! passing this now have tested thoroughly on OSX and Windows upgrading from all stable branches.

          Show
          Sam Hemelryk added a comment - Ficken awesome! passing this now have tested thoroughly on OSX and Windows upgrading from all stable branches.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Oh oh, I'm test-failing this, just detected I'm getting:

          Switched to branch 'master'
          HEAD is now at 7b27861 Merge branch 'MDL-29912' of git://github.com/stronk7/moodle
          Moodle 2.2dev (Build: 20111019) command line installation program
          PHP Fatal error:  Call to undefined function check_moodle_environment() in /Users/Shared/Jenkins/Home/moodle_integration_git/admin/cli/install.php on line 640
          

          since this landed to master. Surely easy to fix... ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Oh oh, I'm test-failing this, just detected I'm getting: Switched to branch 'master' HEAD is now at 7b27861 Merge branch 'MDL-29912' of git: //github.com/stronk7/moodle Moodle 2.2dev (Build: 20111019) command line installation program PHP Fatal error: Call to undefined function check_moodle_environment() in /Users/Shared/Jenkins/Home/moodle_integration_git/admin/cli/install.php on line 640 since this landed to master. Surely easy to fix... ciao
          Hide
          Sam Hemelryk added a comment -

          Good spotting Eloy - indeed I totally forgot about CLI and did not test it!

          I've just had a quick shot at this myself and can confirm the issue. Normal install functions perfectly - however CLI install fails with the noted error.
          I also attempted to add the missing include into admin/cli/install.php however that lead to further problems as the environment code is now attempting to check the DB is unicode ($DB not being setup at this point).

          PHP Fatal error: Call to a member function setup_is_unicodedb() on a non-object in /private/var/www/integration/lib/environmentlib.php on line 828

          Fatal error: Call to a member function setup_is_unicodedb() on a non-object in /private/var/www/integration/lib/environmentlib.php on line 828

          Looks like this may require more attention.

          Show
          Sam Hemelryk added a comment - Good spotting Eloy - indeed I totally forgot about CLI and did not test it! I've just had a quick shot at this myself and can confirm the issue. Normal install functions perfectly - however CLI install fails with the noted error. I also attempted to add the missing include into admin/cli/install.php however that lead to further problems as the environment code is now attempting to check the DB is unicode ($DB not being setup at this point). PHP Fatal error: Call to a member function setup_is_unicodedb() on a non-object in /private/var/www/integration/lib/environmentlib.php on line 828 Fatal error: Call to a member function setup_is_unicodedb() on a non-object in /private/var/www/integration/lib/environmentlib.php on line 828 Looks like this may require more attention.
          Hide
          Sam Hemelryk added a comment -

          One more minor thing that I just noticed as well:

          Notice: Undefined variable: release in /private/var/www/integration/admin/renderer.php on line 70

          Show
          Sam Hemelryk added a comment - One more minor thing that I just noticed as well: Notice: Undefined variable: release in /private/var/www/integration/admin/renderer.php on line 70
          Hide
          Sam Hemelryk added a comment -

          And one more.... the following occurs when trying to run cli upgrade:

          PHP Fatal error: Class 'plugin_manager' not found in /private/var/www/integration/admin/cli/upgrade.php on line 109

          Fatal error: Class 'plugin_manager' not found in /private/var/www/integration/admin/cli/upgrade.php on line 109

          Sorry about missing the CLI tests!

          Show
          Sam Hemelryk added a comment - And one more.... the following occurs when trying to run cli upgrade: PHP Fatal error: Class 'plugin_manager' not found in /private/var/www/integration/admin/cli/upgrade.php on line 109 Fatal error: Class 'plugin_manager' not found in /private/var/www/integration/admin/cli/upgrade.php on line 109 Sorry about missing the CLI tests!
          Hide
          Eloy Lafuente (stronk7) added a comment - - edited

          Tim,

          if you think this is going to get longer than expected I'd propose to:

          • revert the install / upgrade CLI scripts to last week status (keeping the rest).
          • create folloup regression issue to be fixed along the week.

          and then proceed with weekly release process. The alternative is to revert the whole thingy but I don't think we need to be so drastic in this case.

          Of course, if you think the fix is easy... that's the best path.

          For your consideration...ciao

          Show
          Eloy Lafuente (stronk7) added a comment - - edited Tim, if you think this is going to get longer than expected I'd propose to: revert the install / upgrade CLI scripts to last week status (keeping the rest). create folloup regression issue to be fixed along the week. and then proceed with weekly release process. The alternative is to revert the whole thingy but I don't think we need to be so drastic in this case. Of course, if you think the fix is easy... that's the best path. For your consideration...ciao
          Hide
          Tim Hunt added a comment -

          Sam H, I explicitly said above "Test CLI" (see http://tracker.moodle.org/browse/MDL-29474?focusedCommentId=127201&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-127201 and Testing instructions.)

          Petr is fixing this, thanks. I think he will get there quite quickly, so we should wait.

          Show
          Tim Hunt added a comment - Sam H, I explicitly said above "Test CLI" (see http://tracker.moodle.org/browse/MDL-29474?focusedCommentId=127201&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-127201 and Testing instructions.) Petr is fixing this, thanks. I think he will get there quite quickly, so we should wait.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          LOL, Tim, and they have been tested (by Jenkins) and failed, hence here we are, no worries anybody.

          Show
          Eloy Lafuente (stronk7) added a comment - LOL, Tim, and they have been tested (by Jenkins) and failed, hence here we are, no worries anybody.
          Hide
          Petr Škoda added a comment -

          here is the fix for all 3 CLI scripts: https://github.com/skodak/moodle/tree/w42_MDL-29474_m22_fix
          please note AMOS needs to be notified there is a new string in install

          Show
          Petr Škoda added a comment - here is the fix for all 3 CLI scripts: https://github.com/skodak/moodle/tree/w42_MDL-29474_m22_fix please note AMOS needs to be notified there is a new string in install
          Hide
          Petr Škoda added a comment -

          hehe, "I explicitly said test CLI" - I was bitten by this a few times recently and every time I promised to myself that next time I will test everything properly...

          Show
          Petr Škoda added a comment - hehe, "I explicitly said test CLI" - I was bitten by this a few times recently and every time I promised to myself that next time I will test everything properly...
          Hide
          Petr Škoda added a comment -

          I have added a commit that updates the list of strings necessary for install

          Show
          Petr Škoda added a comment - I have added a commit that updates the list of strings necessary for install
          Hide
          Petr Škoda added a comment -

          grrr, I am fixing some more issues in admin/index.php

          Show
          Petr Škoda added a comment - grrr, I am fixing some more issues in admin/index.php
          Hide
          Petr Škoda added a comment -

          done, there is one new TODO in place where we should display a list on non-standard plugins during initial install

          Show
          Petr Škoda added a comment - done, there is one new TODO in place where we should display a list on non-standard plugins during initial install
          Hide
          Tim Hunt added a comment -

          +1 from me for Petr's changes.

          Show
          Tim Hunt added a comment - +1 from me for Petr's changes.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          pushed, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - pushed, thanks!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          automated cli install and upgrade from 20 and 21 working without error now. Thanks!

          I'm going to test web install/upgrade with and without one unmet dependency. And also manual cli install / upgrade with an without the dep.

          Then will look admin/plugins and environment pages. Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - automated cli install and upgrade from 20 and 21 working without error now. Thanks! I'm going to test web install/upgrade with and without one unmet dependency. And also manual cli install / upgrade with an without the dep. Then will look admin/plugins and environment pages. Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Note: Just found MDL-29972 when running CLI install, does not affect this issue. I continue.

          Show
          Eloy Lafuente (stronk7) added a comment - Note: Just found MDL-29972 when running CLI install, does not affect this issue. I continue.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Web tests completed ok:

          • Upgrade from 2.1 with and without dependency problem.
            • environment page on upgrade
            • plugins overview page on upgrade
            • environment page @ admin
            • plugins overview @ admin
          • Install 2.2 with and without dependency problem.
            • environment page on upgrade
            • plugins overview page on upgrade
            • environment page @ admin
            • plugins overview @ admin

          Q: Unmet dependencies on install are not detected at all. Is this expected behavior?

          Show
          Eloy Lafuente (stronk7) added a comment - Web tests completed ok: Upgrade from 2.1 with and without dependency problem. environment page on upgrade plugins overview page on upgrade environment page @ admin plugins overview @ admin Install 2.2 with and without dependency problem. environment page on upgrade plugins overview page on upgrade environment page @ admin plugins overview @ admin Q: Unmet dependencies on install are not detected at all. Is this expected behavior?
          Hide
          Petr Škoda added a comment -

          Yes, I tried to solve it too at the same time, but the task was a bit bigger so I left a TODO there, hopefully next week...

          Show
          Petr Škoda added a comment - Yes, I tried to solve it too at the same time, but the task was a bit bigger so I left a TODO there, hopefully next week...
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Thanks!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          CLI test completed ok:

          • Upgrade from 2.1 with and without dependency problem.
          • Install 2.2 with and without dependency problem.

          Unmet dependencies are detected ok, both on install & upgrade (although no details are shown).

          So I think that we can consider this passed knowing that:

          1) Web install dependencies check must be installed (TODO).
          2) CLI does no show details about the unmet checks.

          +1 ?

          Show
          Eloy Lafuente (stronk7) added a comment - CLI test completed ok: Upgrade from 2.1 with and without dependency problem. Install 2.2 with and without dependency problem. Unmet dependencies are detected ok, both on install & upgrade (although no details are shown). So I think that we can consider this passed knowing that: 1) Web install dependencies check must be installed (TODO). 2) CLI does no show details about the unmet checks. +1 ?
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Paaaaaaaaasssssssssssiiiiiiiiiinnnnnnnnnngggggggg!

          Thanks all!

          Show
          Eloy Lafuente (stronk7) added a comment - Paaaaaaaaasssssssssssiiiiiiiiiinnnnnnnnnngggggggg! Thanks all!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Done, your delicious hacks have been sent upstream, many thanks!

          Closing as fixed, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Done, your delicious hacks have been sent upstream, many thanks! Closing as fixed, ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Oh, oh, just experimented this: MDL-29978 (linked as regression)

          Show
          Eloy Lafuente (stronk7) added a comment - Oh, oh, just experimented this: MDL-29978 (linked as regression)
          Hide
          Akin Delamarre added a comment -

          Is there any chance that this dependency code will be added to later sub versions of 2.0 and 2.1?

          Show
          Akin Delamarre added a comment - Is there any chance that this dependency code will be added to later sub versions of 2.0 and 2.1?
          Hide
          Tim Hunt added a comment -

          Akin, this dependancy code is not essential. It is just a nice thing for administrators, to help them avoid mistakes. It was perfectly possible to have plugins that depended on each other before Moodle 2.2. You just had to document it in the install instructions for your plugin, and administrators had to read it.

          So, basically, this won't be back-ported.

          Show
          Tim Hunt added a comment - Akin, this dependancy code is not essential. It is just a nice thing for administrators, to help them avoid mistakes. It was perfectly possible to have plugins that depended on each other before Moodle 2.2. You just had to document it in the install instructions for your plugin, and administrators had to read it. So, basically, this won't be back-ported.

            People

            • Votes:
              1 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: