Magento2: The bin/magento setup:upgrade Command will Enable Non-Enabled modules

[astorm]

Preconditions

1. Magento installed via composer --create-project method, confirmed 2.1.3, 2.1.5, 2.1.6

Steps to reproduce

1. Create a new module in app/code ($ pestle.phar magento2:generate:module AbcCorp Testbed 0.0.1
2. Run php bin/magento setup:upgrade
3. Examine app/etc/config.php

Expected result

1. AbcCorp_Testbed should not appear in app/etc/config.php

Actual result

1. AbcCorp_Testbed appears in app/etc/config.php, and is enabled

It's a reasonable developer expectation that setup:update should not enable modules that were not previously enabled. This bug came from real world interactions with working developers who hit this bug. This bug caused unexpected Setup Upgrade code to run, and resulted in lost real world working billable hours.

[alankent]

The reason for the current behavior is:

• If you install Magento for the first time, you expect all the modules to be turned on by default.
• If you purchase and install a new extension, you expect it to be installed and turned on by default.
• If you upgrade from say 2.1 to 2.2 and there are new modules important to CE, you expect them turned on by default.

Bottom line is I think there are more use cases where you want new modules turned on by default than turned off by default. Is it in this case more of a question of "if I am half way through creating something under app/code, I don't want it turned on until I have finished, otherwise things break". That is more setup:upgrade ideally should not recognize it as a module at all (don't add to config.php until its complete). Then when its "done", then turn it on. (And no, I don't have a definition of "done" in my mind yet.)

[alankent]

To be more precise, do you think the problem is modules are being picked up and turned on by default, or is the problem that the module is being recognized as module before it is ready for use?

It would be great to get a bit better insight into the original root cause of the problem. I can see that using 'Composer require' and then having upgrade not turn on a module (you have to do extra steps) would also be confusing to people (especially for CE upgrades). So it would be helpful to understand the original problem as to why someone wanted a new module turned off. (My example was I had a half built module that did not load yet - caused bin/magento to break for me. So I can believe there is a problem here worth thinking about.)

[lfolco]

I think that it's just not clear that running setup:upgrade will enable any module that happens to live in app/code that had not been explicitly enabled/disabled. I discovered this when I downloaded a module to take a look at it, put it in app/code (I had an IDE project open), and then ran setup:upgrade for a different module, and all of a sudden the downloaded module was enabled and the database scripts were run.

As a developer, I would prefer to explicitly enable/disable modules rather than have a catch-all mechanism. Or a flag on setup:upgrade --disable-auto-enable or something along those lines.

[korostii]

It would be nice to see the following:

• just the composer modules (placed under vendor) enabled by default (since you must've explicitly installed those via composer already, and something published through composer is more likely to be ready to use)
• and have the local ones under app disabled until an explicit bin/magento module:enable command is executed (since these are the user's own handwritten modules, and he knows better when they're ready).

Surely, such a separation can be done?

[astorm]

@alankent

1. A user adds a module to their app/code folder but does not enable it (because they don't want it enabled)

2. Developer does something unrelated to enabling modules.

3. Yet, somehow, a module is enabled

That's just wrong behavior. Your system fails to honor the implicit contract of the module:enable and module:disable commands. Your system fails to honor the intentions of its users.

If you install Magento for the first time, you expect all the modules to be turned on by default.

No. I, and other users, would not expect a module that's not part of Magento to be enabled by the installer. I'd expect the installer to know which modules it needed and install them, and then potentially offer to install any extensions it sees but doesn't know about.

If you purchase and install a new extension, you expect it to be installed and turned on by default.

No. I, and other users, would expect whatever system I used to install the extension to also enabled the extension I just installed, and only that extension.

If you upgrade from say 2.1 to 2.2 and there are new modules important to CE, you expect them turned on by default.

No. I, and other users, would expect the upgrade process to look at the list of extensions currently enabled, as well as any new extensions added by 2.2, and enable them.

Those three points are just -- weird. To have made an expedient choice with some side effects like this is one thing, and a part of software development. But to double down on that choice's negative side effects, to sell that it's the right behavior, as though the problem can't be fixed?

Just weird.

[bvboas]

I think @alankent is right here.

How many times did we tell Windows to index the file we just copied from an external drive?

How many times did we install an app on a MAC by drag-n-drop and didn't expect macOS to associate an file extension?

How many times did we created an email account and went to the options to enabled it?

I don't think is reasonable to expect that when you place code inside a dir that is used as extension location, to nothing happen when you say to the software. "Please update".

I think the current behavior is the expected one. Additional options can be added to exclude packages, or similar options...

[lfolco]

Currently, the only way to make sure that non-enabled (but not disabled) modules do not run their setup scripts is to enable the module, then disable it. At the very least, it would be nice to have the option to immediately disable a module without having to enable it first (currently you get a "No modules were changed." message).

[bh-ref]

I encountered the same problem some time ago: https://github.com/magento/magento2/issues/2622

We switched our workflow since then: if there is a Magento update with new modules, we run setup:upgrade on our development systems which adds the module to app/etc/env.php. If we decide to not use the module, we let the module entry there, but turn it from 1 to 0:

<?php
return array (
  'modules' => 
  array (
    ....
    'Legit_Module' => 1,
    'Unwanted_Module' => 0,
   ...
  ),
);

If we deploy this code base to the productive environments, the update / setup scripts won't be executed. We use this workflow for Magento updates and extensions we maybe want to be enabled on development systems, but not on the productive systems (so we maintain different app/etc/env.php).

But to be honest, I do not know how well this works with updates via composer, we always use the compressed archives for updating Magento.

And I totally see why developers see the current behavior of app/etc/env.php as weird. I seems it boils down to the question: who is installing modules? A technically savvy user or someone who is thankful that he has not to take more actions for the module to be enabled?

[rparsi-commer]

Is is possible to disable a custom module via its' etc/module.xml file?

[magento-engcom-team]

@astorm, thank you for your report.
This seems to be correct Magento behavior. Please refer to the Community Forums or the Magento Stack Exchange site for advice or general discussion about this.
Otherwise you may submit Pull Request with the suggested changes.

[korostii]

This seems to be correct Magento behavior.

With all due respect, that's an overstatement. Current behavior is very far from perfect.

Please refer to ... for advice ...

IMHO telling Alan Storm (of all people) to seek an "advice" on Magento issues should be considered an insult.

Otherwise you may submit Pull Request with the suggested changes.

Make up your mind already. If you agree that a PR fixing this bug should be accepted, then the current behavior cannot be correct.

non-issue

What's that supposed to mean?
Considerning the comments above, this bug is being a real issue to some real people.
Marking it as a non-issue underminds the effort people have put into investigating possible solutions to this issue.

[orlangur]

@astorm

A user adds a module to their app/code folder but does not enable it (because they don't want it enabled)
Developer does something unrelated to enabling modules.
Yet, somehow, a module is enabled

setup:upgrade IS related to enabling modules.

have the local ones under app disabled until an explicit bin/magento module:enable command is executed (since these are the user's own handwritten modules, and he knows better when they're ready).

No, thanks, I don't want to run this command manually for each module. Why even put modules not supposed to be enabled into app/code?

Whoever will work on this issue please don't change default behavior but make a configuration option for those who need another behavior.

According to my understanding there could be some interaction added to setup:upgrade command with some yellow message "New modules detected AbcCorp_Testbed, AbcCorp_Testtable, AbcCorp_Testchair. Do you wish to enable them?" and ability to enable all with one character typed or choose status one-by-one.

[korostii]

I don't want to run this command manually for each module

Why even add a bunch of modules into app/code at once?
If you do that very often, could you please provide a use case explaining why is this needed?
The only case I think of is: when you pull code from repository and don't have an app/etc/env.php file (yet) because it's been added to .gitignore.

Also, wouldn't the existing bin/magento module:enable --all option solve that case for you?

[lfolco]

I discovered this issue when I downloaded a third party module with the intention of looking at it / evaluating it within the context of an IDE/existing Magento project. I put it in app/code when I downloaded it, and then when doing other work that involved setup:upgrade, I noticed that the module was enabled. This was not what I was expecting; I had expected that, once I looked over the code, I could then decide if I wanted to enable it or not. When working with third party code, you often want to inspect it more carefully before running it. And, it's much easier to inspect it within a Magento project where an IDE can pick up any glaring issues, etc.

Not sure if this use case warrants a feature change, but I do think it's rather unexpected, especially given that there is already a command to enable/disable modules explicitly.

[orlangur]

@lfolco

downloaded a third party module with the intention of looking at it / evaluating it within the context of an IDE/existing Magento project

Obviously you shouldn't do it as without intention to run it is much safer to put in separate folder where you just have code of your project plus vendors.

Current behavior of setup:upgrade is something you should be already aware of if used Magento at least a month. It is much more appropriate scenario to keep only needed modules and simply run setup:upgrade on staging after each deploy.

Some note could be added into setup:upgrade that new modules will be enabled or some flag setup:upgrade --do-not-enable-new-modules but existing behavior should not be changed to not break existing workflows.

[korostii]

existing behavior should not be changed to not break existing workflows

Any sane workflow should use a list of explicit commands rather than depend on an obscure piece of undocumented functionality that might be considered a bug and removed in the future.
IMHO both employing bin/magento module:enable --all and versionizing your config.php are better options than having setup:upgrade fulfill a totally unrelated (and surprising to many) responsibility by default.

[orlangur]

It is absolutely clear and useful functionality. That's why default system behavior MUST NOT be changed but some switches could be introduced to use different behavior.

If it is not clear to anyone, there is no problem with documenting it.

IMHO both employing bin/magento module:enable --all and versionizing your config.php are better options than having setup:upgrade

Both require additional actions which is why current default behavior MUST NOT be spoiled by any "improvements" which reduce its usefulness.

[lfolco]

It's not clear, though, as evidenced by this ticket. It doesn't say anywhere in the dev docs that this behavior occurs. There is a distinct enable/disable command, and a setup:upgrade command. There is not indication outside of experience that setup:upgrade would incorporate functionality from the enable/disable command.

I may be working on a module in app/code that I don't want to enable right away (I'm doing install scripts, etc.), so now I have to go in and disable a module that I had never enabled? This is not a clear workflow at all.

Can you give me an example of a workflow where this functionality is expected? Outside of doing core development, that is, which is the only scenario I can see where auto-enabling modules would save time.

[orlangur]

I may be working on a module in app/code that I don't want to enable right away

Just keep it out of the code or stash such changes before running setup:upgrade. Always keep your working tree clean to avoid undesired consequences.

Useful workflow which may be documented and must be preserved:

• update code
• run setup:upgrade
• result: all non-disabled modules are upgraded, new ones are enabled

We are using it all the time working on custom modules development so that there is no need to put config.php under VCS or communicate which modules were just added.

[korostii]

We are using it all the time

Just because you personally like this workflow more doesn't mean everyone should use it.

Could you please explain what's the advantage of your approach over the "need to put config.php under VCS" approach, if you're keeping all the modules enabled at all times anyway (I mean, you won't need to disable any module locally if you prefer to "keep it out of the code or stash such changes")?

Besides, the steps you describe above will work just as fine if you simply put config.php under VCS.

[orlangur]

Many of us may like this behavior thus it should be carefully evaluated before changing. For example, template path hints in Admin UI were first introduced as a bug not existed in M1 but later it was found useful and claimed as a feature.

1) config.php is redundant junk in current workflow
2) leads to conflicts when stored under VCS
3) your store may have implicit module dependencies if order is always static due to config.php

[korostii]

config.php is redundant junk in current workflow

Except some people prefer keeping incomplete modules in VCS, disabling them via config.php.

leads to conflicts when stored under VCS
your store may have implicit module dependencies if order is always static due to config.php

IMHO These are a bit out of scope of current discussion. Please consider reopening and upvoting https://github.com/magento/magento2/issues/8479 if you agree that current config.php mechanics needs to be improved.

Also, with the current config.php behavior I'd rather have implicit dependencies stored in a versioned config.php because otherwise you might suddenly happen to have a different load order on _some_ of the instances (e.g. staging or production) causing issues that are hard to reproduce and identify using a different (e.g. local) instance.

[orlangur]

some of the instances (e.g. staging or production) causing issues that are hard to reproduce and identify using a different (e.g. local) instance.

No problem, just disable such modules explicitly not breaking setup:upgrade for others.

Issue you mentioned simply does not contain any appropriate criteria as I explained in it.

some of the instances (e.g. staging or production) causing issues that are hard to reproduce and identify using a different (e.g. local) instance.

It's a good thing to not rely on any particular order. In case of problem simply get a config.php to check from problematic env.

[lfolco]

The fact that this ticket was opened a year and a half after Magento 2 was released is indicative of the fact that this behavior is not expected; the very command name (upgrade) hides the fact that it also enables.

I'm curious as to which workflow is more common in day-to-day Magento development: users who are working on multiple modules that need to be upgraded and enabled all at once, or those that are working on one at a time? This ticket has eight up votes, one down vote, so it seems that the proposed change is preferred. How is it decided to make such a feature change? Would it be decided upon submitting a pull request?

[orlangur]

Votes are irrelevant here as happy users of current workflow just don't read this ticket.

I believe implementation approach should be defined prior to PR creation, will raise this topic among maintainers.

[korostii]

No problem, just disable such modules explicitly not breaking setup:upgrade for others.

setup:upgrade is irrelevant in this case. Having a versioned config.php eliminates both the need to communicate which modules have do be disabled with other members of the team and the need to disable them explicitly on the other environments.

Having config.php unversioned AND having setup:upgrade unexpectedly change module status is a recipe for getting your Magento into a broken state.
Consider a module being added, enabled just like that, and then deleted from VCS while you still have it in your local config.php
This will lead to a cryptic Setup version for module 'Bla_blabla' is not specified error message on the storefront while the bin/magento module:disable Bla_blabla command will tell you Unknown module(s): Bla_blabla forcing you to remove it from config.php manually.

Please don't think your workflow is a silver bullet which will fit everyone's needs perfectly. Because it's not.

Issue you mentioned simply does not contain any appropriate criteria as I explained in it.

Well it's certainly a better place starting point to discuss load order issues than this ticket. That was the whole point of that ticket and there were a few good points made there.

For example, one could suggest having module load order computed at runtime and cached elsewhere. That would leave the config.php with the sole purpose of tracking which modules are enabled, and it will be totally okay then for the module order in it to be changed to alphabetic. Thus storing it in VCS would have no drawbacks whatsoever.

It's a good thing to not rely on any particular order. In case of problem simply get a config.php to check from problematic env.

Again, accessing a different environment isn't any easier than having config.php versioned in the first place. Module load sequence shouldn't be environment-specific and thus there's no reason to have it excluded from VCS.

[magento-engcom-team]

Hi @astorm. Thank you for the report. We are moving your feature request to the special project. You can track this issue here: https://github.com/magento-engcom/community-features/issues/25

[astorm]

@magento-engcom-team That means Magento won't be changing/fixing this, correct?

[korostii]

@astorm, well, their readme seems to confirm your guess:
https://github.com/magento-engcom/community-features/blob/master/README.md

Magento Features Development is an Initiative to Allows Community Memebers Join to Development of Magento Features

[orlangur]

@astorm @korostii does MSI project mean Magento is not implementing Multi-Store Inventory? :)

[korostii]

Hi @orlangur,

Can tell if it's a honest question or you're just trolling.
Please keep in mind that according to the Code of Conduct, trolling is considered "unacceptable behavior" and thus I would suggest you to be "focusing on what is best for the community" instead.
I suspect that you already know that, but, in any case, MSI seems to be a separate project with a different scope of work. And it is not relevant to this discussion.
If you have any additional insider information regarding @astorm's question, please feel free to put it in the open explicitly rather vaguely hinting about it.

I, for one, don't have any insider information thus I have to resort to publicly available information (posted above) and my personal viewpoints and speculation.
IMHO, @magento-engcom-team seems to attempt to offload every non-essential task to community members. At the same time, they seem to be mostly ignoring the feedback from said community members when it comes to making decisions (please do correct me if this impression is wrong).
Like, for example, when making a decision whether some issue is considered a bug, a suggested improvement, or (as in this case, for some unclear reason) a "feature request".

To be perfectly clear, personally I think that arbitrary moving some part of the unfinished issues to a separate repository, community forum, or a closed state does little to none good to the overall project progress.

[orlangur]

(as in this case, for some unclear reason) a "feature request"

Reason is perfectly clear. Current behavior could be documented but as original request is to change it, it is definitely a change request of currently intended behavior.

MSI will become a part of core at some point by the way.

arbitrary moving some part of the unfinished issues to a separate repository, community forum, or a closed state does little to none good to the overall project progress.

As it was explained hundreds of times, this repository is not a right place for feature requests, let's just concentrate on quality here and manage only bug reports. This new repository seems to be a place where a work by Community members on new features may be performed and managed well. Much better than moving discussions to Community Forums as to me.

[korostii]

Current behavior could be documented but as original request is to change it, it is definitely a change request of currently intended behavior.

This is probably a bit off-topic. I'll just say that I really don't see where are you getting the "intended" part from and how is it relevant. I mean, the feature itself is already present, effectively making this issue a "bug report" or a "suggested improvement" at best.

This new repository seems to be a place where a work by Community members on new features may be performed and managed well. Much better than moving discussions to Community Forums as to me.

Yes, of course, it is better than forums. Personally, I do not know what's the benefit over having them right here in the same repository - but you're quite right, it's certainly an improvement.

However, that wasn't the question asked by Alan. The question, as far as I understand, was whether Magento (the company) plans to put any effort into the tasks which got moved to that repository.

Again, if you have any additional insider information regarding @astorm's question, please feel free to put it in the open explicitly.

[mattwellss]

Since config.php can't be reliably used to disable a module, I'll suggest composer's replace as an alternative. Here's a completely hypothetical example composer.json of a "remove modules" library:

{
    "name": "remove/these-modules",
    "version": "1.0.0",
    "replace": {
        "dotmailer/dotmailer-magento2-extension": "*",
        "temando/module-shipping-m2": "*"
    }
}

This module (remove/these-modules) must be installable. I'm currently tracking it in the main repository and using a path repository, but a separate repo and vcs works just fine, too.

Once composer require remove/these-modules is run, the modules listed in replace will be removed and the new module "installed".

n.b. as shown, multiple modules can be listed in replace.

Hope this is helpful.