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

Module uninstall does not work with composer #17780

Closed
Thundar opened this issue Aug 24, 2018 · 17 comments
Closed

Module uninstall does not work with composer #17780

Thundar opened this issue Aug 24, 2018 · 17 comments
Labels
Component: Setup Fixed in 2.2.x The issue has been fixed in 2.2 release line Fixed in 2.3.x The issue has been fixed in 2.3 release line Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release

Comments

@Thundar
Copy link
Contributor

Thundar commented Aug 24, 2018

Preconditions

  1. Magento 2.2.5

Steps to reproduce

  1. Install Magento with composer (2.2.x)
  2. Install Sample Data with bin/magento sampledata:deploy
  3. Complete Sample Data installation: bin/magento setup:upgrade
  4. Remove one package: bin/magento module:uninstall Magento_CmsSampleData

Expected result

The expecteded result is that the result matches the guide and its sample:
https://devdocs.magento.com/guides/v2.2/install-gde/install/cli/install-cli-uninstall-mods.html

  1. vendor/module folder should be removed
  2. composer.lock file should be updated,
    or, to have an update in the guide to fit the actual result.

Actual result

screen shot 2018-08-31 at 13 33 33

Missing expected result (matching the guide):

Removing code from Magento codebase:
Loading composer repositories with package information
Updating dependencies (including require-dev)
Removing magento/module-cms-sample-data (100.2.0)
Removing Magento/CmsSampleData
Writing lock file
Generating autoload files

That means that with the next magento setup:upgrade the module will be reinstalled.
In Framework\Composer\Remove.php, function remove, line 49, the option --no-update is set, that does not match with what the guide says.

@magento-engcom-team magento-engcom-team added the Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed label Aug 24, 2018
@magento-engcom-team
Copy link
Contributor

magento-engcom-team commented Aug 24, 2018

Hi @Thundar. Thank you for your report.
To help us process this issue please make sure that you provided the following information:

  • Summary of the issue
  • Information on your environment
  • Steps to reproduce
  • Expected and actual results

Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:

@magento-engcom-team give me {$VERSION} instance

where {$VERSION} is version tags (starting from 2.2.0+) or develop branches (2.2-develop +).
For more details, please, review the Magento Contributor Assistant documentation.

@Thundar do you confirm that you was able to reproduce the issue on vanilla Magento instance following steps to reproduce?

  • yes
  • no

@Thundar
Copy link
Contributor Author

Thundar commented Aug 24, 2018

@magento-engcom-team give me 2.2.5 instance

@magento-engcom-team
Copy link
Contributor

Hi @Thundar. Thank you for your request. I'm working on Magento 2.2.5 instance for you

@magento-engcom-team
Copy link
Contributor

Hi @Thundar, here is your Magento instance.
Admin access: https://i-17780-2-2-5.engcom.dev.magento.com/admin
Login: admin Password: 123123q
Instance will be terminated in up to 3 hours.

@Thundar
Copy link
Contributor Author

Thundar commented Aug 24, 2018

@magento-engcom-team I cannot reproduce the error in that instance as I have not access to this instance's console, so I cannot install any additional test module, nor remove it (also, setup folder is "secured" with a 500 error)

@hostep
Copy link
Contributor

hostep commented Aug 24, 2018

The --no-update flag was added in scope of #5797 (commit: 4c5c1f9#diff-6ad856a2714512cc350bda789df0ab92), but I think that was done incorrectly and they should have added the --no-update-with-dependencies flag like I initially mentioned in that ticket.

Anyway, I've never actually used the module:uninstall command since then, because it makes no sense to me that Magento is trying so desperately to integrate with Composer.
Composer is supposed to be a stand-alone utility which is meant for developers, with which they can manage their dependencies. Magento should preferably stay away from this and I think the docs might need to be updated that if you manage your dependencies with composer, you should also just use composer to remove your modules instead of using module:uninstall, but that's just my opinion :)

@Thundar
Copy link
Contributor Author

Thundar commented Aug 24, 2018

I have not tried it, but I think that --no-update-with-dependencies does not exist anymore, or maybe it could be removed soon: https://getcomposer.org/doc/03-cli.md#remove

@hostep
Copy link
Contributor

hostep commented Aug 24, 2018

Hmm, probably just undocumented, running composer 1.7.2:

$ composer remove --help
Usage:
  remove [options] [--] <packages> (<packages>)...

Arguments:
  packages                           Packages that should be removed.

Options:
      --dev                          Removes a package from the require-dev section.
      --no-progress                  Do not output download progress.
      --no-update                    Disables the automatic update of the dependencies.
      --no-scripts                   Skips the execution of all scripts defined in composer.json file.
      --update-no-dev                Run the dependency update with the --no-dev option.
      --update-with-dependencies     Allows inherited dependencies to be updated with explicit dependencies. (Deprecrated, is now default behavior)
      --no-update-with-dependencies  Does not allow inherited dependencies to be updated with explicit dependencies.
      --ignore-platform-reqs         Ignore platform requirements (php & ext- packages).
  -o, --optimize-autoloader          Optimize autoloader during autoloader dump
  -a, --classmap-authoritative       Autoload classes from the classmap only. Implicitly enables `--optimize-autoloader`.
      --apcu-autoloader              Use APCu to cache found/not-found classes.
  -h, --help                         Display this help message
  -q, --quiet                        Do not output any message
  -V, --version                      Display this application version
      --ansi                         Force ANSI output
      --no-ansi                      Disable ANSI output
  -n, --no-interaction               Do not ask any interactive question
      --profile                      Display timing and memory usage information
      --no-plugins                   Whether to disable plugins.
  -d, --working-dir=WORKING-DIR      If specified, use the given directory as working directory.
  -v|vv|vvv, --verbose               Increase the verbosity of messages: 1 for normal output, 2 for more verbose output and 3 for debug

Also: https://github.com/composer/composer/blob/1.7.2/src/Composer/Command/RemoveCommand.php#L45

But you don't need it if you just want to remove a module with composer, then composer remove <vendor>/<module-name> is perfectly fine, the --no-update-with-dependencies was only needed for the issue described in #5797

@ghost ghost self-assigned this Aug 28, 2018
@ghost
Copy link

ghost commented Aug 28, 2018

@Thundar, thank you for your report.
This is not a bug. Please refer to Magento 2 documentation

This command does not, however, remove the module’s code from the Magento file system.

@ghost ghost closed this as completed Aug 28, 2018
@ghost ghost added the non-issue label Aug 28, 2018
@Thundar
Copy link
Contributor Author

Thundar commented Aug 28, 2018

@engcom-backlog-pb
The part you quoted refers to non composer modules (see previous and next phrases, not using composer), while composer's module are supposed to be removed by the command (and it makes sense, because app/code is committed, while vendor/ is not).
You can find more detailed documentation later in the page:

  1. Removes code from the codebase using composer remove.

Plus, the example output shows the removal from vendor/ folder and .lock file writing.

You can try the same command of the example on Magento's core modules: you'll have a different result.

Please tell me if I have to open a different and more detailed issue.

@ghost ghost reopened this Aug 31, 2018
@ghost ghost removed the non-issue label Aug 31, 2018
@ghost
Copy link

ghost commented Aug 31, 2018

@Thundar please provide more clear steps to reproduce. I'm getting
Vendor_Module is not an installed composer package
error when trying to module:uninstall any module installed by Composer.

@Thundar
Copy link
Contributor Author

Thundar commented Aug 31, 2018

@engcom-backlog-pb
Ok. This is the procedure to uninstall Magento_CmsSampleData. Please note that you will get the same same result with any (installed) module, is it maintained from Magento or from third parties.

  1. Install Magento with composer (2.2.x)
  2. Install Sample Datas with bin/magento sampledata:deploy
  3. Complete Sample Data installation: bin/magento setup:upgrade
  4. Remove one package: bin/magento uninstall Magento_CmsSampleData

Actual Result:
screen shot 2018-08-31 at 13 33 33

Missing expected result (matching the guide):

Removing code from Magento codebase:
Loading composer repositories with package information
Updating dependencies (including require-dev)
Removing magento/module-cms-sample-data (100.2.0)
Removing Magento/CmsSampleData
Writing lock file
Generating autoload files

@hostep and I tracked the technical cause of the bug in the above comments.

@ghost
Copy link

ghost commented Aug 31, 2018

@Thundar, thank you for your clarification.
We've acknowledged the issue and added to our backlog.

@ghost ghost added Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Component: Setup and removed Progress: needs update labels Aug 31, 2018
@ghost ghost removed their assignment Aug 31, 2018
@Thundar
Copy link
Contributor Author

Thundar commented Aug 31, 2018

@engcom-backlog-pb Thanks

@Thundar
Copy link
Contributor Author

Thundar commented Sep 10, 2018

@hostep I can confirm that the option --no-update-with-dependencies works as desidered.

@sidolov
Copy link
Contributor

sidolov commented Sep 22, 2018

Hi @Thundar. Thank you for your report.
The issue has been fixed in #18002 by @Thundar in 2.2-develop branch
Related commit(s):

The fix will be available with the upcoming 2.2.8 release.

@sidolov sidolov added the Fixed in 2.2.x The issue has been fixed in 2.2 release line label Sep 22, 2018
@sidolov sidolov closed this as completed Sep 22, 2018
@sidolov sidolov added the Fixed in 2.3.x The issue has been fixed in 2.3 release line label Sep 26, 2018
@sidolov
Copy link
Contributor

sidolov commented Sep 26, 2018

Hi @Thundar. Thank you for your report.
The issue has been fixed in #18205 by @mage2pratik in 2.3-develop branch
Related commit(s):

The fix will be available with the upcoming 2.3.1 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Setup Fixed in 2.2.x The issue has been fixed in 2.2 release line Fixed in 2.3.x The issue has been fixed in 2.3 release line Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release
Projects
None yet
Development

No branches or pull requests

4 participants