Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New dependencies installation fails for some Extras (MIGX, SEO tab, SEO Pro) #12666

Closed
DESIGNfromWITHIN opened this issue Sep 23, 2015 · 13 comments

Comments

@DESIGNfromWITHIN
Copy link

When using the new dependencies feature in MODX 2.4.0-pl to install MIGX, SEO Pro or SEO Tab, The extra's get listed but when you click install they do not get installed.

The only way to install them is via the 'normal' way, after this they get listed correctly as installed under dependencies tab.

I tried different names, but no go for these three extras. Others do work fine...

@joeke
Copy link
Contributor

joeke commented Jan 6, 2016

I've tested this with Modx 2.4.2 and I've got the same issue. I've dug into the problem and the issue seems to be with spaces in the dependency package name. See issue #55 at the SEO Tab repo for more info: Sterc/SEOTab#55

@joeke
Copy link
Contributor

joeke commented Oct 10, 2016

@opengeek I've had a talk with you (on Slack) about this, and as you pointed out it seems to be an issue in the modx extras repository when using dependencies with a space in the package name.. Are there any plans / ideas on fixing this? Would love to help out, but I don't think the Extras repository code is publicly available, is it?

@Mark-H
Copy link
Collaborator

Mark-H commented Feb 9, 2017

Closing as this likely needs to be resolved in the provider/extras site.

@Mark-H Mark-H closed this as completed Feb 9, 2017
@opengeek
Copy link
Member

opengeek commented Feb 9, 2017

Package names never have spaces in them; if you submit them that way, they are cleansed. They have to match the transport filename. I'm a little confused as to what the problem really is here.

@opengeek opengeek reopened this Feb 9, 2017
@opengeek
Copy link
Member

opengeek commented Feb 9, 2017

@joeke I recall the conversation but, unfortunately, I don't recall the conversation details. If you can provide more details here that might help. But again, packages should never contain a space in the name. They should always be lowercase and not contain a dash character, just like the transport filename rules.

@joeke
Copy link
Contributor

joeke commented Feb 10, 2017

@opengeek I've managed to reproduce the issue, here are the steps:

  1. Build new transport zip for any extra, but add SEO Pro to 'requires' array (key: 'SEO Pro', value: '>=1.0.4'). (please note that I use the package name here, because that's how the checkDependencies and checkDownloadedDependencies functions in modtransportpackage.class.php handle the dependencies.)
  2. Install the newly created transport zip in a clean modx install (at least without SEO Pro installed).
  3. The installer screen shows the Dependencies tab, and it shows SEO Pro, with a download button.
  4. Click on the download button, the request fails with following error message:
    {"success":true,"message":"Could not download and create transport package with signature: ","total":0,"data":[],"object":[]}.

My request URL is
/connectors/index.php?_dc=1486719097990&action=workspace%2Fpackages%2Fdependency%2Fdownload&signature=stercseo-2.0.0-alpha&name=SEO%20Pro&constraints=%3E%3D1.0.4&HTTP_MODAUTH=modx587f83ab408533.79723877_1589d854ac0eda2.61248522

Note that when I install SEO Pro before installing my created transport zip (in my clean modx install), the installation works fine. So the issue only occurs when using dependencies that are not installed.

@Jako
Copy link
Collaborator

Jako commented Feb 10, 2017 via email

@joeke
Copy link
Contributor

joeke commented Feb 10, 2017

@Jako No that won't work. The checkDependencies function (which is used for the 'requires' array in a build) uses the package_name, which in this case is 'SEO Pro' (see screenshot).
screen shot 2017-02-10 at 11 08 38

The checkDependencies function is here: https://github.com/modxcms/revolution/blob/2.x/core/model/modx/transport/modtransportpackage.class.php#L468

@Jako
Copy link
Collaborator

Jako commented Feb 10, 2017

Stupid design decision then.

The query should be expanded with an or condition that checks the string before the first - sign in the signature column in that table.

@joeke
Copy link
Contributor

joeke commented Feb 10, 2017

@Jako I don't think the problem is with the checkDepencies functions, I believe those are only used when building the package.. and that works fine. I think the problem is somewhere in the dependency download class which is used on the install screen for downloading the package (https://github.com/modxcms/revolution/blob/2.x/core/model/modx/processors/workspace/packages/dependency/download.class.php)
or perhaps the error occurs in the extras repository.

@Jako
Copy link
Collaborator

Jako commented Jan 11, 2019

It was maybe a combination of several issues:

@alroniks
Copy link
Collaborator

@Jako Did you fixed described issue or some point still actual? If all fixed could close the issue as your confirmation?

@Jako Jako closed this as completed Feb 13, 2019
@Jako
Copy link
Collaborator

Jako commented Feb 13, 2019

It is fixed with that query change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants