magento / devdocs

[Deprecated] Magento Developer Documentation
Open Software License 3.0
674 stars 1.76k forks source link

Documentation is missing an important step! #8690

Closed Eddcapone closed 2 months ago

Eddcapone commented 3 years ago

General issue

Description:

Feedback on page: /guides/v2.4/config-guide/deployment/pipeline/example/shared-configuration.html

I was running into what I thought was a bug and created an issue, because the documentation does miss a very important step.

In Step 3 it says that we have to execute:

To update your build system:

Log in to your build system as, or switch to, the Magento file system owner. Change to the build system’s Magento root directory. Pull the changes to app/etc/config.php from source control.

The Git command follows:

git pull mconfig m2.2_deploy Compile code:

php bin/magento setup:di:compile After code has been compiled, generate static view files:

php bin/magento setup:static-content:deploy -f Check the changes into source control.

The Git command follows:

git add -A && git commit -m "Updated files on build system" && git push mconfig m2.2_deploy

It misses the information that you need to execute php bin/magento setup:upgrade before php bin/magento setup:di:compile. This can lead to style changes not getting detected and compiled.

Possible solutions:

Add missing step

m2-assistant[bot] commented 3 years ago

Hi @Eddcapone. Thank you for your report. To help us process this issue please make sure that you provided sufficient information.

Please, add a comment to assign the issue: @magento I am working on this


hostep commented 3 years ago

Hi @Eddcapone, I think you have misunderstood what all these commands do based on incorrect comments in https://github.com/magento/magento2/issues/32149 unfortunately.

  1. bin/magento setup:di:compile has nothing to do with compiling static assets.
  2. bin/magento setup:upgrade already runs both setup:di:compile and setup:static-content:deploy if you don't specify the --keep-generated flag. In theory it shouldn't be needed to run these 2 commands again after setup:upgrade
  3. The problem you are running into is probably some left over files from a previous setup:static-content:deploy run. It's probably better to remove all the files in var/view_preprocessed/* and pub/static/* (except for the .htaccess file that is) before you run setup:static-content:deploy
  4. The documentation you refer to is about a build server, which generally does not have access to the database of the shop and running bin/magento setup:upgrade will fail when no database is available.

What most people do in their deploy scripts, it to deploy Magento to a new empty directory on a server each time and when the deploy is finished, create a symlink from the document root to that new release. That way you don't need to clean up some directories yourself because you started from a blank slate.

We have been asking to the devdocs team to document something like that but that issue got closed with an incorrect reason: https://github.com/magento/devdocs/issues/419. After some quick searching the devdocs, I don't find a page where it documents what you are looking for. Which is unfortunate, maybe some guide should get created where it explains in detailed steps how people can deploy a shop without a build server? The flow like how https://github.com/davidalger/capistrano-magento2/ does it for example?

Eddcapone commented 3 years ago

"bin/magento setup:di:compile has nothing to do with compiling static assets."

Then why does the documentation says I need to execute it? Look at Step 4.

"The documentation you refer to is about a build server, which generally does not have access to the database"

I followed the instructions to create the build server. In the documentation it says that it does not need a database, but I was not able to make the build system work without a database, I will test if it works without setup:upgrade if I delete the folders instead. You say that I have to delete var/view_preprocessed/* and pub/static/* but this step is never mentioned in the documentation.

hostep commented 3 years ago

Then why does the documentation says I need to execute it? Look at Step 4.

Because it is needed to generate php files which are needed on production. The step is definitely needed, it's just not used for generating static assets (which is what you were asking for in your opening post).

You say that I have to delete var/view_preprocessed/ and pub/static/ but this step is never mentioned in the documentation.

The documentation assumes you are running this in a clean environment (you start from an empty directory) but doesn't explicitly mention this. Maybe that can get added to make the documentation clearer?

I will test if it works without setup:upgrade if I delete the folders instead

You also need setup:upgrade if you deploy to the final production server in order to run the database migration scripts. But I would suggest you run this as one of the last commands and use the --keep-generated flag so it doesn't re-executes setup:di:compile and setup:static-content:deploy

Also, maybe this comment can help: https://github.com/magento/magento2/issues/29570#issuecomment-675434852

Eddcapone commented 3 years ago

"The documentation assumes you are running this in a clean environment"

This is not true, there is no mention of this in the Assumptions section. Look here.

You also need setup:upgrade if you deploy to the final production server in order to run the database migration scripts.

Am I correct with my assumption that I need to execute php bin/magento setup:upgrade --keep-generated only on the production server after git pull?

I tested it by delting the static content before recompiling, instead of executing setup:upgrade that worked too 👍

hostep commented 3 years ago

This is not true, there is no mention of this in the Assumptions section.

That's why I said:

... but doesn't explicitly mention this. Maybe that can get added to make the documentation clearer?


Am I correct with my assumption that I need to execute php bin/magento setup:upgrade --keep-generated only on the production server after git pull?

It's a bit too complicated to explain, but sometimes you don't need to execute setup:upgrade at all if only certain files got changed since the last time you ran it. But that's an expert optimisation (which is probably also not explained on devdocs), to be safe, you should execute it each time code has changed.

I tested it by delting the static content before recompiling, instead of executing setup:upgrade that worked too

Good to hear this 🙂

mrtuvn commented 3 years ago

The only confuse me is in here https://devdocs.magento.com/guides/v2.4/config-guide/deployment/pipeline/build-system.html Doc said build system doesnot need database. I think it's not true. We need database for build system(staging, production, development) Should we remove that statement. Especially when run some basic command related with config

command setup:upgrade is optional depend on scenario. Yes i agreed with hostep here. When we change something in config or install new module. keep-generated option mentioned here too https://devdocs.magento.com/guides/v2.4/config-guide/deployment/pipeline/technical-details.html#production-system

hostep commented 3 years ago

I have no personal experience with a build server since we build on the same server as where we publishing the new version of our code.

But I'm very confident that a true build server (as they assume you have in that documentation) doesn't need access to the database. You only need database access when you publish new code to the server which actually runs the code.

It has something to do with storing store-related config data in you app/etc/config.php file so that the SCD command knows which locale's to deploy if I understand it correctly and then it doesn't need database access.

Eddcapone commented 3 years ago

It's a bit too complicated to explain, but sometimes you don't need to execute setup:upgrade at all

I understand you, I could just create two deploy scripts, one without and one with the statement and only use the setup one if I install/remove/update extensions.

That's why I said:

... but doesn't explicitly mention this. Maybe that can get added to make the documentation clearer?

Oh sorry, my fault

mrtuvn commented 3 years ago

Seem docs have updated can you recheck that ? Give docs team a feedback on new content

Santoshziffity commented 3 years ago

@magento I am working on this

dshevtsov commented 2 months ago

The topic was migrated. If the issue is still relevant, please feel free to reopen it.