Feature PR-5.2-dev Pending

User tests: Successful: Unsuccessful:

avatar brbrbr
brbrbr
25 Jun 2024

Maybe mark this with RFC?

Summary of Issues

Deletion of package

with --path processPathInstallation would delete the package file if it's placed in the tmp_path.
There are two flaws with this:

  1. If the file is in tmp_path it is deleted, otherwise it's not. This is inconsistent.
  2. Extension:install --path` never downloads the package itself, so it should not delete with is not its.

So --path should only clean up the extracted files. (--path does not accept a URL as parameter, it never downloads a file)

Inconsistent names with the web interface

In the web interface we have 'URL', 'folder' and 'package'  (and web, that one is not applicable for the CLI)
'URL' can be found in extension:install, no problems there.

--path is slightly ambiguous, at a first glance you might expect this to point to a folder. Only after careful reading of the help information, it is clear that it must be a package. 

An option to install from a folder is missing at all

To be more consistent, extension:install should support 'folder' and 'package' as well

Summary of Changes

 - adding an option --package to point to a zip file (processPackageInstallation)
 - Adding an option --folder to point to a folder with extracted files (processFolderInstallation)
 - Modifying --path to accept a folder or a file (B/C). Delegating the work to processPackageInstallation or processFolderInstallation
 - Package installation only deletes the extracted files. Not the original package.
 - Minor changes to the help info, extension:install can update as well.- Added short versions of the parameters

Testing Instructions

--url is omitted.
Ensure the CLI command is executed with the right permissions. Installation fails otherwise without a clear messsage.

Actual result BEFORE applying this Pull Request

 - php joomla extension:install --path <zipfile> - works
 - php joomla extension:install --path <tmp_dir/zipfile> - works - file deleted
 - php joomla extension:install --path <folder> - fails

Expected result AFTER applying this Pull Request

 - php joomla extension:install --path <zipfile> - works (B/C)
 - php joomla extension:install --path <folder> - works
 - php joomla extension:install --path <tmp_dir/zipfile> - works - zipfile not deleted 
 
 - php joomla extension:install --package | -p <folder> - works
 - php joomla extension:install --folder | -f <folder> - works
 
 - php joomla extension:install --package | -p <folder> - fails
 - php joomla extension:install --folder | -f <zipfile> - fails

avatar brbrbr brbrbr - open - 25 Jun 2024
avatar brbrbr brbrbr - change - 25 Jun 2024
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 25 Jun 2024
Category Libraries
avatar brbrbr brbrbr - change - 25 Jun 2024
Labels Added: PR-5.2-dev
avatar brbrbr brbrbr - change - 27 Jun 2024
Labels Added: Feature
avatar degobbis
degobbis - comment - 24 Aug 2024

Hmm, changing the expected behaviour would be a B/C break, wouldn't it?

So the change from
php joomla extension:install --path <tmp_dir/zipfile> - works - file deleted to
php joomla extension:install --path <tmp_dir/zipfile> - works - zipfile not deleted is, in my opinion, a B/C break.

Basically, I think the default behaviour should always be to clean up after yourself. If this doesn't happen, I think it's more of a bug.

If something is not to be cleaned up after processing, an option such as “--doNotClean” would be the better approach, wouldn't it?

avatar brbrbr
brbrbr - comment - 24 Aug 2024

Basically, I think the default behaviour should always be to clean up after yourself. If this doesn't happen, I think it's more of a bug.

Since the --path option does not download the file, the command should not be responsible for cleanup as yourself.

The webclients administrator 'folder' install does not delete the used folder, neither.

This is different with the (proposed) --url option which downloads the file, therefor is responsible for cleanup as yourself and should clean up.

Besides the deletion of the file depends on the location of the file.

avatar HLeithner
HLeithner - comment - 2 Sep 2024

This pull request has been automatically rebased to 5.3-dev.

avatar HLeithner HLeithner - change - 2 Sep 2024
Title
ExtensionInstallCommand more consistent with administrator
[5.3] ExtensionInstallCommand more consistent with administrator
avatar HLeithner HLeithner - edited - 2 Sep 2024

Add a Comment

Login with GitHub to post a comment