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

Setting alternative web/public dir is inconsistent/fails #2461

Closed
m-vo opened this issue Oct 26, 2020 · 8 comments · Fixed by #3084
Closed

Setting alternative web/public dir is inconsistent/fails #2461

m-vo opened this issue Oct 26, 2020 · 8 comments · Fixed by #3084
Assignees
Labels
Milestone

Comments

@m-vo
Copy link
Member

m-vo commented Oct 26, 2020

Affected version(s)

Contao 4.9

Description

see discussion #2183 (comment)

Contao uses two sources for the configured web/public directory: The config from extra.symfony-public-dir / extra.public-dir (root composer.json) and the config key contao.web_dir (core-bundle).

The bundle config defaults to %kernel.project_dir%/web. It's used in quite some places (templating, images, combiner, ...). The current composer script handler uses the entry from the root composer.json and defaults to web (relative path). So you need to keep these things in sync otherwise either one fails.

Possible solution

Imo we should only only use the config in our code and its default should be:

  1. root composer.jsonextra.public-dir
  2. if it does not exist: root composer.jsonextra.symfony-public-dir
  3. if it does not exist: %kernel.project_dir%/web
@leofeyer
Copy link
Member

Does it work in Contao 4.9?

@m-vo
Copy link
Member Author

m-vo commented Oct 26, 2020

No, same there. Or rather: 4.4 isn't affected from the same problem as nothing is read from the composer.json there. I adjusted the initial description.

For reference:

  • The config was added in 8f4c7f3.
  • The script handler was adjusted in c1d93b5.

@aschempp
Copy link
Member

There are two configurations, because there are two separate environments. Contao does not know about the Composer configuration, and Composer does not know about Contao.

Maybe we should add a compiler pass that sets the contao.web_dir similar to symfony/symfony#29533?

@m-vo
Copy link
Member Author

m-vo commented Oct 28, 2020

👍 The difference being, that Symfony does not have a dedicated config option, right? (I don't see a compiler pass in the referenced PR, maybe I'm missing sth.)

We would still need to make sure that you did not overwrite the config and use a setting the composer.json. Also we must only use the config in our code. Correct?

@leofeyer leofeyer added the bug label Oct 30, 2020
@leofeyer leofeyer added this to the 4.9 milestone Oct 30, 2020
@leofeyer leofeyer added the up for discussion Issues and PRs which will be discussed in our monthly Mumble calls. label Nov 2, 2020
@leofeyer
Copy link
Member

As discussed in Mumble on November 19th, we want to set contao.web_dir from the composer.json configuration, following the Symfony way. If there is no symfony-web-dir in the composer.json, we should fall back to public in the core-bundle and overwrite this with web in the managed-edition (BC).

@leofeyer leofeyer modified the milestones: 4.9, 4.11 Nov 19, 2020
@leofeyer leofeyer added feature and removed bug up for discussion Issues and PRs which will be discussed in our monthly Mumble calls. labels Nov 19, 2020
@fritzmg
Copy link
Contributor

fritzmg commented Nov 19, 2020

For reference, the following parameters can be set in Symfony Flex: https://github.com/symfony/flex/blob/e38520236bdc911c2f219634b485bc328746e980/src/Flex.php#L879-L885

@m-vo
Copy link
Member Author

m-vo commented Feb 8, 2021

This issue is marked as feature against 4.12 whereas #2597 as bug against 4.9. Can we consolidate these tickets and decide on a strategy (bug vs feature) 😄?

@leofeyer leofeyer added the up for discussion Issues and PRs which will be discussed in our monthly Mumble calls. label Feb 11, 2021
@leofeyer
Copy link
Member

As discussed in Mumble on March 11, it should work in Contao 4.9 if you set the web directory in both the composer.json and the app configuration. If this is not the case, we should fix it.

The new feature for Contao 4.12 would be what we have discussed in the last call.

@leofeyer leofeyer removed the up for discussion Issues and PRs which will be discussed in our monthly Mumble calls. label Mar 11, 2021
@leofeyer leofeyer linked a pull request Jun 14, 2021 that will close this issue
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants