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

Use/support public/ folder for Contao Managed Edition #1088

Closed
fritzmg opened this issue Dec 11, 2019 · 19 comments · Fixed by #3084
Closed

Use/support public/ folder for Contao Managed Edition #1088

fritzmg opened this issue Dec 11, 2019 · 19 comments · Fixed by #3084
Labels
Milestone

Comments

@fritzmg
Copy link
Contributor

fritzmg commented Dec 11, 2019

Since Symfony 4.4, in a regular Symfony application you would have support for the following structure:

App Bundle
config/ config/
public/ public/
src/ src/
templates/ templates/
translations/ translations/

Basically, everything in the bundle is consistent with the application itself.

After this PR is merged, the Contao Managed Edition would support the following:

App Bundle
config/ config/
contao/ contao/
web/ public/
src/ src/
templates/ templates/
translations/ translations/

So only the web/ folder would be inconsistent with the regular Symfony application and bundle structure.

I know this has been discussed in the past and it was decided against it - mostly to not cause more confusion within the community (I think?). But due to the recent PR I would like to bring it up and discuss it again :)

@aschempp
Copy link
Member

As far as I know, you can use a different folder and configure the application accordingly? There should be a property in the contao bundle config.

@fritzmg
Copy link
Contributor Author

fritzmg commented Dec 11, 2019

Yep, there is.

contao:
  web_dir: %kernel.project_dir%/web

However, the question is, if the default should be changed to public/, for consistency with current Symfony best-practices and standards.

@contaoacademy
Copy link

Does this mean that the domain root would have to be set to /mypath/public?

@leofeyer
Copy link
Member

I am up for renaming the web/ folder to public/ in Contao 4.9. Not sure about the upgrade path though - how would you upgrade from Contao 4.4 to 4.9 then?

@contao/developers /cc

@Toflar
Copy link
Member

Toflar commented Dec 16, 2019

We can't. It would require to change the DirectoryIndex of your server configuration. I don't see any value. At all.

@leofeyer
Copy link
Member

Renaming the directory has the same benefit as deprecating the /app/Resources/config directory in favour of the /config directory. We did not have to do this, either, however we wanted to adjust to the Symfony standards. 🤷‍♂

@Toflar
Copy link
Member

Toflar commented Dec 16, 2019

What? We're talking web/index.php to public/index.php here, right? How is that related to /app/Resources?

@leofeyer
Copy link
Member

leofeyer commented Dec 16, 2019

Both are non-required changes that do not have to be done, because the "old" way works just fine. Still we now support the config/ directory and we have deprecated app/Resources/config/, which has no benefit except adjusting to the Symfony standards.

But switching from web/ to public/ suddenly needs more benefits than that?

@Toflar
Copy link
Member

Toflar commented Dec 16, 2019

app/Resources/config to config can be migrated automatically. The DirectoryIndex of the webserver cannot.

@leofeyer
Copy link
Member

That's true. Still we should think of an upgrade path so we can change this in a future version, don't you think?

@Toflar
Copy link
Member

Toflar commented Dec 16, 2019

I don't think we can. The only way that would work would be to have the public directory alongside the web directory and that again would mean we have to map everything that's generated into web or public vice-versa.

The only option for me here is a hard break. Either we do that in 4.9 LTS or 4.10.

Also note that we cannot just rename a config value because unfortunately there's still quite a few places where web is hardcoded. Examples include:

  • Calendar::generateFeed()
  • News::generateFiles()

@aschempp
Copy link
Member

This cannot be changed in 4.9 because of lots of (potential) BC breaks. We can discuss this for 4.10 imho, maybe someone can come up with a clever way (like symlinking web to public 😉 )

@contaoacademy
Copy link

Is this change really strategically so great? A lot of blog posts, videos and forum posts would no longer fit.

Especially for inexperienced users there will be a lot of confusion...

@leofeyer
Copy link
Member

+1 for a hard break in Contao 4.10 then.

@m-vo
Copy link
Member

m-vo commented Dec 17, 2019

I'd rather be able to configure it. So for new projects it would be public to be in line with the symfony docs but web for existing ones. I don't see any benefit in the BC break. 🤷‍♂️

If we want to change the default, I'd still vote for a config option so that noone is forced to change their server and deployment settings.

@Toflar
Copy link
Member

Toflar commented Dec 17, 2019

You can configure it already. Fixing the mentioned hardcoded ones should be an easy-pick.

@leofeyer
Copy link
Member

I have removed the hardcoded paths in #1116.

leofeyer added a commit that referenced this issue Dec 18, 2019
Description
-----------

See #1088 (comment)

Commits
-------

14e0f3c Replace "web/" with "contao.web_dir"
leofeyer added a commit to contao/calendar-bundle that referenced this issue Dec 18, 2019
Description
-----------

See contao/contao#1088 (comment)

Commits
-------

14e0f3c4 Replace "web/" with "contao.web_dir"
leofeyer added a commit to contao/news-bundle that referenced this issue Dec 18, 2019
Description
-----------

See contao/contao#1088 (comment)

Commits
-------

14e0f3c4 Replace "web/" with "contao.web_dir"
@kroerig
Copy link

kroerig commented Jan 5, 2020

I'd rather be able to configure it. So for new projects it would be public to be in line with the symfony docs but web for existing ones. I don't see any benefit in the BC break. 🤷‍♂

If we want to change the default, I'd still vote for a config option so that noone is forced to change their server and deployment settings.

Perhaps you could implement a migration tool in Contao-Manager.

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

leofeyer commented Mar 12, 2020

As discussed in Mumble on March, 12th, the Contao Manager knows how the user has named its parent directory and could store this in the container. Then it would make no difference how the users name their public directory, as it would be set correctly by the Manager (similar to how it is set to web in the current Symfony Flex recipe).

The default in Contao 4.10 should be public. If a web folder exists, public should be a symlink to web.

@leofeyer leofeyer added feature and removed up for discussion Issues and PRs which will be discussed in our monthly Mumble calls. labels Mar 12, 2020
@leofeyer leofeyer added this to the 4.10 milestone Mar 12, 2020
@leofeyer leofeyer modified the milestones: 4.10, 4.11 Jul 21, 2020
@leofeyer leofeyer removed this from the 4.11 milestone Jan 7, 2021
@leofeyer leofeyer added this to the 4.12 milestone Jan 7, 2021
@leofeyer leofeyer linked a pull request Jun 14, 2021 that will close this issue
leofeyer added a commit that referenced this issue Jun 18, 2021
Description
-----------

| Q                | A
| -----------------| ---
| Fixed issues     | Implements #1088
| Docs PR or issue | TODO

This PR renames the `web` folder to `public` as discussed here: #1088 (comment)

@contao/developers How do we want to implement backwards compatibility? I see two options:

1. Add a check in the `Configuration` class: If `contao.web_dir` is `public` and there is no `public` folder but a `web` folder, change the value to `web` automatically.

2. Make the `GenerateSymlinkCommand` generate a symlink from `public` to `web` if a `web` folder exists.

I am in favor of solution no. 1, because I find having two different public folders confusing. WDYT?

Commits
-------

c398fba Rename the "web" folder to "public"
a56bfaf Use /public instead of /web everywhere
40c6b92 Dynamically set the default value
c8bd306 Add the BC layer to the commands, too
39261c2 Fix the CI chain
leofeyer added a commit to contao/core-bundle that referenced this issue Jun 18, 2021
Description
-----------

| Q                | A
| -----------------| ---
| Fixed issues     | Implements #1088
| Docs PR or issue | TODO

This PR renames the `web` folder to `public` as discussed here: contao/contao#1088 (comment)

@contao/developers How do we want to implement backwards compatibility? I see two options:

1. Add a check in the `Configuration` class: If `contao.web_dir` is `public` and there is no `public` folder but a `web` folder, change the value to `web` automatically.

2. Make the `GenerateSymlinkCommand` generate a symlink from `public` to `web` if a `web` folder exists.

I am in favor of solution no. 1, because I find having two different public folders confusing. WDYT?

Commits
-------

c398fbaa Rename the "web" folder to "public"
a56bfaf5 Use /public instead of /web everywhere
40c6b92f Dynamically set the default value
c8bd3060 Add the BC layer to the commands, too
39261c27 Fix the CI chain
leofeyer added a commit to contao/manager-bundle that referenced this issue Jun 18, 2021
Description
-----------

| Q                | A
| -----------------| ---
| Fixed issues     | Implements #1088
| Docs PR or issue | TODO

This PR renames the `web` folder to `public` as discussed here: contao/contao#1088 (comment)

@contao/developers How do we want to implement backwards compatibility? I see two options:

1. Add a check in the `Configuration` class: If `contao.web_dir` is `public` and there is no `public` folder but a `web` folder, change the value to `web` automatically.

2. Make the `GenerateSymlinkCommand` generate a symlink from `public` to `web` if a `web` folder exists.

I am in favor of solution no. 1, because I find having two different public folders confusing. WDYT?

Commits
-------

c398fbaa Rename the "web" folder to "public"
a56bfaf5 Use /public instead of /web everywhere
40c6b92f Dynamically set the default value
c8bd3060 Add the BC layer to the commands, too
39261c27 Fix the CI chain
@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.

7 participants