installModule() does not actually install the module when run the 1st time, but only downloads it

[ivangretsky]

As the title says) I've tested it from the profile import and console. In both cases after the first run the $rm->installModule only downloads module files. You need to run the same script the second time to make it actually installed.

[ivangretsky]

@BernhardBaumrock , I've tested it from a standalone script as well as from the module console. Same thing.

I've tried to debug the issue. I was monitoring the db changes.

After the 1st run the module path is added to the record in modules table with the class field equal to .Modules.site/modules/. But no special record for that module appears.

After the 2nd run the correct record for the module is there.

Could it be related to recent changes in modules caching?

[BernhardBaumrock]

Hi @ivangretsky thx for the report, I can confirm that behaviour and that it is a bug. The installModule method works fine if the module files exist on the file system but it fails if it has to download them.

I don't know why the download fails. I've tried to refactor that method to this code:

  /**
   * Download module from url
   *
   * @param string $url
   * @return void
   */
  public function downloadModule($name, $url)
  {
    $http = new WireHttp();
    $toFile = $this->wire->config->paths->cache . "module.zip";
    $http->download($url, $toFile);
    $unzip = $this->wire->files->unzip($toFile, $this->wire->config->paths->siteModules);
    $this->wire->files->unlink($toFile);
    $newPath = $this->wire->config->paths->siteModules . $name;
    $this->wire->files->rename(
      $this->wire->config->paths->siteModules . $unzip[0],
      $newPath
    );
    return $newPath;
  }

But it's the same behaviour.

Maybe @gebeer has an idea?

[ivangretsky]

@BernhardBaumrock , can you confirm that it did work as expected at some point in the past?

[BernhardBaumrock]

I can't confirm that 100% because it's usually not a big deal in real world scenarios. I'll explain what I mean:

Imagine you want to install a new module. What I usually do is to download that module via GUI and add one line to my Site.module.php

$rm->installModule("MyNewModule");

Downloading the module will add all files in /site/modules and those files will be pushed to my github repo for that project. Then also the Site.module.php will be pushed to github and once those files are deployed to the staging or live system everything will be installed properly by the migration called by the deployment.

I'm not saying that this means that it's not an issue, just trying to explain why it's not a big deal for me or has not been.

But I think it has worked, because the profile migrations install some modules from an external url and that has been a one click experience for a long time. I'll keep an eye open for that the next time I use that feature.

[gebeer]

Hi @ivangretsky thx for the report, I can confirm that behaviour and that it is a bug. The installModule method works fine if the module files exist on the file system but it fails if it has to download them.

I don't know why the download fails. I've tried to refactor that method to this code:

  /**
   * Download module from url
   *
   * @param string $url
   * @return void
   */
  public function downloadModule($name, $url)
  {
    $http = new WireHttp();
    $toFile = $this->wire->config->paths->cache . "module.zip";
    $http->download($url, $toFile);
    $unzip = $this->wire->files->unzip($toFile, $this->wire->config->paths->siteModules);
    $this->wire->files->unlink($toFile);
    $newPath = $this->wire->config->paths->siteModules . $name;
    $this->wire->files->rename(
      $this->wire->config->paths->siteModules . $unzip[0],
      $newPath
    );
    return $newPath;
  }

But it's the same behaviour.

Maybe @gebeer has an idea?

Different behaviour for me: First run: files do not get downloaded, module not installed. Second run: downloaded and installed. This is the downloadModule method in my install: https://github.com/gebeer/RockMigrations/blob/da449aff93dba3fe3672adfc4383179ed048bacf/RockMigrations.module.php#L1562

But I discovered that the culprit might be in https://github.com/baumrock/RockMigrations/blob/950476ff27256581c63467a6347929cae809cb2d/RockMigrations.module.php#L2259 When I change that line to read:

$pathExists = ($path and $this->wire->files->exists($path));

then it works. EDIT: it also works if I replace it with: $pathExists = $path && $this->wire->files->exists($path);

The original expression for $pathExists returned the actual path and not a bool as I would have expected. But as to why, I have no idea. EDIT: might be because that particular install runs on PHP7.4?

@ivangretsky @BernhardBaumrock You might want to try that fix and confirm if it works.

[BernhardBaumrock]

Wow @gebeer thank you, I can confirm that this fix works for me on php8.1!

[gebeer]

Wow @gebeer thank you, I can confirm that this fix works for me on php8.1!

Would be interesting to know why && or wrapping the expression in () gives a different result.

[gebeer]

GPT4 says: In PHP, the && and and operators are used to perform logical AND operations, however, they differ in their precedence. The && operator has higher precedence compared to the and operator. This difference in precedence affects how expressions are evaluated when these operators are used.

When the and operator is used, due to its lower precedence, the expression is evaluated like so: ($pathExists = $path) and $this->wire->files->exists($path); In this scenario, $path is assigned to $pathExists first, and then the and operation is performed but its result is not stored anywhere.

Now, in the modified code snippet: $pathExists = $path && $this->wire->files->exists($path); With the && operator's higher precedence, the expression is evaluated like so: $pathExists = ($path && $this->wire->files->exists($path));

I was not aware of that difference between and and &&

[BernhardBaumrock]

I've pushed that fix to the dev branch.

Wow, thx for that lesson @gebeer !

Thx for the issue report @ivangretsky

[ivangretsky]

Great job, @gebeer ! Thanks for the quick fix @BernhardBaumrock !

[ivangretsky]

Seems to be the precedence issue. Now I know why I always use && )))) https://www.daniweb.com/posts/jump/1814435

[ivangretsky]

Hello, @BernhardBaumrock !

We need to reopen this one. The issue is not really solved. As @gebeer wrote above he had a different issue. And the solution we came up with only solves his part. But I actualy have another issue.

It is here

The $module is null as the module does not get installed. Its files are there, but the install method checks if the class we need to install is in the installableFiles array. And it isn't.

I came up with the solution that seems to work. There might be some better way to do it, but here is the code that did it for me:

  public function installModule($name, $conf = [], $options = [])
  {
    ...

      if ($pathExists) {
        // module files are in place -> install the module
        $this->modules->refresh(); // <--- THIS ONE!
        $module = $this->modules->install($name, ['force' => $opt->force]);
        if ($module) $this->log("Installed module $name");
        else $this->log("Tried to install module $name but failed");
      }
    }
  ...

[BernhardBaumrock]

Hi @ivangretsky thx. Could you please check with $this->refresh() instead of your $this->modules->refresh() ?

[ivangretsky]

This one works too. Made a PR earlier, feel free to delete it.

[gebeer]

Hello, @BernhardBaumrock !

We need to reopen this one. The issue is not really solved. As @gebeer wrote above he had a different issue. And the solution we came up with only solves his part. But I actualy have another issue.

It is here

The $module is null as the module does not get installed. Its files are there, but the install method checks if the class we need to install is in the installableFiles array. And it isn't.

I came up with the solution that seems to work. There might be some better way to do it, but here is the code that did it for me:

  public function installModule($name, $conf = [], $options = [])
  {
    ...

      if ($pathExists) {
        // module files are in place -> install the module
        $this->modules->refresh(); // <--- THIS ONE!
        $module = $this->modules->install($name, ['force' => $opt->force]);
        if ($module) $this->log("Installed module $name");
        else $this->log("Tried to install module $name but failed");
      }
    }
  ...

That would have been my suggestion for a fix, too. Seems like a legit thing to do :-)

[BernhardBaumrock]

Thx, I've merged it