Change behavior of modules <depends> node - any downsides?

Question

Current sitution:

If one of you extensions depends on another, that isn't active or not installed it breaks page load and you'll see error report page.

<?xml version="1.0"?>
<config>
    <modules>
        <My_Module>
            <active>true</active>
            <codePool>local</codePool>
            <depends>
                <Non_Existing />
            </depends>
        </My_Module>
    </modules>
</config>

Desired behavoir:

If dependency is missing, just do not load the extension, but load the page correctly.

The related code is this: https://github.com/OpenMage/magento-mirror/blob/magento-1.8/app/code/core/Mage/Core/Model/Config.php#L835-L881

I played around a bit and modified code that it does not load the extension:

/**
 * Sort modules and check depends
 *
 * @param array $modules
 * @return array
 */
protected function _sortModuleDepends($modules)
{
    foreach ($modules as $moduleName => $moduleProps) {
        $unset = false; // added
        $depends = $moduleProps['depends'];
        foreach ($moduleProps['depends'] as $depend => $true) {
            if ($moduleProps['active'] && ((!isset($modules[$depend])) || empty($modules[$depend]['active']))) {
                // replaced Mage::throwException
                Mage::log(
                    Mage::helper('core')->__('Module "%1$s" requires module "%2$s".', $moduleName, $depend)
                );
                // added
                unset($modules[$moduleName]);
                $unset = true;

            }
            if (!$unset) { // added
                $depends = array_merge($depends, $modules[$depend]['depends']);
            }
        }
        if (!$unset) { // added
            $modules[$moduleName]['depends'] = $depends;
        }
    }
    $modules = array_values($modules);

    $size = count($modules) - 1;
    for ($i = $size; $i >= 0; $i--) {
        for ($j = $size; $i < $j; $j--) {
            if (isset($modules[$i]['depends'][$modules[$j]['module']])) {
                $value       = $modules[$i];
                $modules[$i] = $modules[$j];
                $modules[$j] = $value;
            }
        }
    }

    $definedModules = array();
    foreach ($modules as $key => $moduleProp) { // added $key
        $unset = false; // added
        foreach ($moduleProp['depends'] as $dependModule => $true) {
            if (!isset($definedModules[$dependModule])) {
                // replaced Mage::throwException
                Mage::log(
                    Mage::helper('core')->__('Module "%1$s" cannot depend on "%2$s".', $moduleProp['module'], $dependModule)
                );
                // added
                unset($modules[$key]);
                $unset = true;
            }
        }
        if (!$unset) { // added
            $definedModules[$moduleProp['module']] = true;
        }
    }

    return $modules;
}

---

Question: it seems to work as intended, but did i've missed something or can I use this safly in production?

---

Update

In addition to Marius answer just some test code ... I created three modules that rewrite Mage_Catalog_Block_Product_View and just outputs a message on getProduct() call.

First one is disabled:

<config>
    <modules>
        <Sr_Disabled>
            <active>false</active>
            <codePool>local</codePool>
        </Sr_Disabled>
    </modules>
</config>

Second one is enabled, but depends on the disabled one:

<config>
    <modules>
        <Sr_Enabled>
            <active>true</active>
            <codePool>local</codePool>
            <depends>
                <Sr_Disabled />
            </depends>
        </Sr_Enabled>
    </modules>
</config>

Third one to test ...

<config>
    <modules>
        <Sr_Test>
            <active>true</active>
            <codePool>local</codePool>
            <!--
            <depends>
                <Sr_Enabled />
            </depends>
            -->
        </Sr_Test>
    </modules>
</config>

I understand your concerns, but I could not find any wrong behavior yet.

For the autoloader you can still check if the module is enabled Mage::helper('core')->isModuleEnabled('Sr_Enabled'), so you can cover the case where module is just disabled, but still present in code pool.

If it's deleted and you want to extends one of its classes you'll get default error message Warning: include(Sr/Enabled/Block/Product/View.php): failed to open stream. Indeed, this would break the site, but this always happens if a class isn't available. This can be solved if you set <depends> node in your module ...

---

Update 2

For ProxiBlues note ... calling a non existing helper from layout it could break the site ...

• so either add a check for isModuleEnabled()
• or edit Mage_Core_Model_Layout::_generateAction

Not tested yet, but it should work ...add a exists/enabled/callable check to:

$args[$key] = call_user_func_array(array(Mage::helper($helperName), $helperMethod), $arg);

Answer 1

In magento 1 the point of the dependency is to tell one module that it cannot function without an other. You should not toy with the way the dependencies are checked. The fact that it works may mean that the declared dependency was not actually a dependency. Or that it was a soft dependency.
Maybe the dependency was determined by one class in the module that extends one class in the dependency. In this case it will work because the autoloader does not care about active or inactive modules. It just searches for the class.
Even in this case it is dangerous. Usually you don't keep disabled modules for long. On the next cleanup if you remove the disabled module you may be in trouble.
Conclusion: do not touch the dependency check just for the sake of a module. Change the module instead