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

Remove version numbers from the extracted project folders when installing manually #6656

Open
klonos opened this issue Jul 12, 2024 · 9 comments · May be fixed by backdrop/backdrop#4831
Open

Comments

@klonos
Copy link
Member

klonos commented Jul 12, 2024

When installing a module manually from GitHub, the URL of the .zip file looks like this:
https://github.com/backdrop-contrib/paragraphs/archive/refs/heads/1.x-1.x.zip

When you install manually via the "Install from a URL" option, you end up with a folder like this:
modules/paragraphs-1.x-1.x.

That is perfectly fine on your local or in hosting where you have direct access to rename folders, but in sandbox situations like Tugboat/Softaculous etc. you are stuck with the version number in the module folder. AFAIK, the modules keep working as expected even with these numbers in the folder names, but this looks messy, and it is more trouble than worth fixing, especially for less technical people.

We should be doing some "sanitization" and try to remove versions from module folder names after extracting the .zip. Doing that as part of the installation would:

  • provide consistency with how the rest of the modules are handled by the Project Installer (no version number suffix in their folders).
  • make it one less step that people have to do manually.
  • save people from all the trouble they'd need to go through in order to fix this when realizing that this has happened after they've enabled the module via the admin UI (having to backup/export configuration, disable/uninstall the module, rename the folder, re-install, and import configuration).
@klonos klonos self-assigned this Jul 12, 2024
@laryn
Copy link
Contributor

laryn commented Jul 12, 2024

Could this be why sometimes people have/had trouble updating a module that was manually installed? I don't remember specifics but I remember there was an issue around that sort of strangeness. (It may have been unrelated and it may also be solved already but just jotting this in case it helps trigger a memory for someone who remembers more about that).

@klonos
Copy link
Member Author

klonos commented Jul 12, 2024

I don't recall anything in the core issue queue, but my memory is not as it used to be. Perhaps it was reported in the forum (which I am not monitoring)? Others may know.

@klonos
Copy link
Member Author

klonos commented Jul 12, 2024

...we could add a check in the status report for mismatching project names (based on their .info file) and respective folder names. Similar to how we check for duplicate modules in the codebase.

@laryn
Copy link
Contributor

laryn commented Jul 12, 2024

I did some searching but couldn't find an issue for that. Maybe as part of this one we can include a test where an older version is installed manually, and then attempt to upgrade via the project installer and see if it works.

@klonos
Copy link
Member Author

klonos commented Jul 12, 2024

That sounds like a good idea. I'm not good at writing tests, so I might need help with that.

@laryn
Copy link
Contributor

laryn commented Jul 12, 2024

Ah ha! #3997

@klonos
Copy link
Member Author

klonos commented Jul 12, 2024

See? ...I told you my memory is not as it used to be ...yet another issue that I filed that I didn't remember 😅 👴🏼

@klonos
Copy link
Member Author

klonos commented Jul 14, 2024

Here's an attempt at fixing this: backdrop/backdrop#4831

Initially, I tried renaming/cleaning up the name of the project folder after it was extracted under the /tmp/update-extraction-* directory, but the amount of changes required in order to do that was too much (and also too messy in various places). It seems that once the project was extracted there, it was "too late" to clean things up and fix problematic folders in various places (in the temp directory, where downloaded projects are cached, and then also in the destination folder where the extracted folder is copied to after all checks etc.). Perhaps there is an easier/tidier way to do that - I tried, so I'll leave that to someone else if that's preferred (cleaning things up after project archive extraction that is).

What I ended up doing instead is this:

  • Extended ArchiverInterface with a renameContents() method, and implemented that (in the ArchiverZip class for now - I left the ArchiverTar class as a todo, see inline comments in the PR). That way, we change the project folder from within the .zip file, before it is even extracted. So the extracted folder name is correct (no version numbers etc.), which means that everything else can remain unchanged. Sorta dealing with the problem at its root, which is the fact that the project folder already contains the version numbers in the downloaded archive (they are added by the packager or GitHub - nothing that Backdrop core is doing wrong there).
  • Abstracted the retrieval and clean up of the project name from version numbers etc. into a new _installer_manager_get_project_from_archive() function, since that was done in at least 2 different places.
  • Cleaned up installer_verify_update_archive():
    • added a couple of additional checks for the .info files to the existing check for core version compatibility:
      • check for missing type
      • check for invalid type
    • slightly refactored the logic so to bail early (no need to be declaring variables that will never be used, nor to try iterating through .info files with the foreach if there are no .info files present in the extracted archive to begin with).

At the moment all tests are passing - only missing function docblocks (pre-existing issue but I'm planning to fix them too regardless). Also planning to add tests for thing that we now know to be causing problems (but as I said, I'll need help with that).

@klonos
Copy link
Member Author

klonos commented Jul 14, 2024

...oh, I'll also test to see if #3997 is related or whether the culprit for this issue here also had something to do with that issue there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment