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

Move core files under /core directory and add symlinks to them #3025

Closed
ghost opened this issue Apr 6, 2018 · 23 comments
Closed

Move core files under /core directory and add symlinks to them #3025

ghost opened this issue Apr 6, 2018 · 23 comments

Comments

@ghost
Copy link

ghost commented Apr 6, 2018

In a similar effort to #2555, what about moving the other core files (see list below) into the /core directory, and then adding symlinks to them in the root directory?

The files I'm referring to include:

  • .editorconfig
  • .gitignore
  • .htaccess
  • index.php
  • README.md
  • robots.txt
  • settings.php
  • profiles

This would make it easier if/when those files are ever updated, but I'm not sure technically if this would be possible, or feasible.

Thoughts?

@ghost ghost added the needs - more feedback label Apr 6, 2018
@opi
Copy link

opi commented Apr 6, 2018

I don't see the point of doing such a change.

@olafgrabienski
Copy link

olafgrabienski commented Apr 6, 2018

In my understanding, such a change could make updating Backdrop CMS to a newer version more clear. According to backdropcms.org/upgrade you have to replace the core directory to update Backdrop. With that information, I asked myself however several times if any of the core files which are not in the core directory may also have been updated.

Not sure if symlinks are easy to handle and if they work in every environment. If not, it may also help to make the update documentation more clear.

@Graham-72
Copy link

When updating a D7 site I have to make sure not to overwrite the existing .htaccess and settings.php files and so I am very glad they are not in BD core. I am in favour of keeping things as they are.

@opi
Copy link

opi commented Apr 6, 2018

When updating a D7 site

IIRC, the release note mention if there are any changes to one of these files. Backdrop may do the same.

@ghost
Copy link
Author

ghost commented Apr 7, 2018

When updating a D7 site I have to make sure not to overwrite the existing .htaccess and settings.php files and so I am very glad they are not in BD core.

That's a good point. What if we updated the documentation to explain that if you need to make changes to them, delete the symlink to /core and copy them into the root directory? That way people that don't customise them can still have them updated automatically, and those who do don't have to worry about them being overwritten...

@findlabnet
Copy link

@BWPanda, sorry, but what do you want to achieve by this proposal? It is not clear to me at all.

@ghost
Copy link
Author

ghost commented Apr 7, 2018

Currently: To update Backdrop you must update/replace the /core directory. You must then check if anything outside of /core has been updated (/profiles, or any of the 7 files I listed in the original post) and, if so, update/replace them too.

Proposed: To update Backdrop you simply update/replace the /core directory. Done.

Symlinks in the root directory to files in /core mean that those files are updated along with core automatically, so it makes updating Backdrop quicker and easier (for you guys too, since you won't have to worry about documenting changes to those files in the release notes).

As mentioned in my second post, if people want/need to edit those core files (e.g. .htaccess), they can delete the symlink to /core's version, and instead copy it into their root directory, make their changes, and not worry about /core updates reverting anything.

Does that explain it better?

@findlabnet
Copy link

IMO, if any of document root files should be updated (but anyway not auto-replaced) by security or any other reason - release notes need contain this information and no more.

Why?

  • any normal minor or even security update is not required changes in such files;
  • are you ready for auto/accidental replacing settings.php and .htaccess (customized per site in any case) by auto/core update - and I sure: you have universal shell script for backup/restore such files which is compatible with any Linux distribution and just works on any shared hosting?
  • even today some people forced by circumstances to work under windows environment for local/staging development - so, after finding any suitable LAMP/LEMP/anything emulator, he/his should search realization of symlinks emulator?
  • how many sites which have similar troubles (TBD if this issue realized) you personally ready to maintain?

With best regards.

@olafgrabienski
Copy link

The (Drupal 7) release note mention if there are any changes to one of these files. Backdrop may do the same.

I've just checked alls the posts on https://backdropcms.org/tags/releases and didn't see any mentions of core file changes there. On https://github.com/backdrop/backdrop/releases such changes were only mentioned rarely, with the exception of the recent critical security updates.

So, I'd suggest in any case

  • to mention in upcoming release notes clearly if any (and which) core files have been changed,
  • to add a note regarding core files to the update documentation on https://backdropcms.org/upgrade.

I guess it's better to open a separate issue for it ... but where?

@jenlampton
Copy link
Member

jenlampton commented Apr 12, 2018

I've updated the documentation page on how to update so it now includes the following:

If any of the files outside the core directory have been changed, they will be explicitly mentioned in the release notes.

edit: I've also updated all our release checklists to specifically include a note about changes to these files.

@ghost
Copy link
Author

ghost commented Apr 12, 2018

I watched the weekly video where @quicksketch suggested adding a line to all release notes stating whether updates were made outside core or not, and I think that's a good idea.

@quicksketch
Copy link
Member

Looks like @jenlampton beat me to it. But here's a follow-up PR: #3031

It's worth restating here that we maintain a list of procedural processes here in the backdrop-issues repository (as Jen linked to above): https://github.com/backdrop/backdrop-issues/tree/master/procedures/docs

Any suggestions/enhancements to our processes can be added to by filing a PR against this repository. Though it's always good to discuss via an issue first (like this one).

@klonos
Copy link
Member

klonos commented Jun 17, 2018

Yeah, although I totally get the replace-core-folder-be-done-with-it intention behind this proposal, my first reaction when I read the title and the summary of this issue was "Oh-oh, scary!" 😅 . Unfortunately, although we could in theory have a set of local.settings.php/prod.settings.php/stage.settings.php files, there is no way to have multiple versions of .htaccess and robots.txt that I know of. I wish!

@opi
Copy link

opi commented Jun 17, 2018

Maybe a bad solution, but what about maintaining some default.htaccess and default.robots.txt ? in a /core subfolder maybe

@jlfranklin
Copy link
Member

Putting in default versions sounds good to me. A core/docs directory would be an ideal place to put them, if we had such a directory. If we add core/docs, we can add other things there, like a Getting Started Guide or a Contrib Developers Primer, both with basic information about Backdrop and links to other resources (Projects page on b.o, api.b.o, Docker page, etc.)

@klonos
Copy link
Member

klonos commented Oct 15, 2018

I am OK with adding default versions of files. This is how imagine things could work, using .htaccess as an example:

  • we do not include a .htaccess file in our codebase
  • we include a default.htaccess (a copy of our current .htaccess)
  • during installation, if there is no .htaccess, we create one by copying default.htaccess
  • default.htaccess could live somewhere inside /core

@ghost
Copy link
Author

ghost commented Oct 16, 2018

during installation, if there is no .htaccess, we create one by copying default.htaccess

What if you add an existing site to BD and there's no installation per se? Wouldn't that be a security issue...?

@klonos
Copy link
Member

klonos commented Oct 16, 2018

What if you add an existing site to BD and there's no installation per se?

...I don't get this. Can you please elaborate @BWPanda?

@ghost
Copy link
Author

ghost commented Oct 16, 2018

@klonos Ok, so when I read what you said about creating the .htaccess file during installation, I wondered what happens when installation doesn't happen (assuming by 'installation' you mean 'install.php')...

For example, I have an existing BD site and database. I setup a new copy of BD (it doesn't have .htaccess in the root dir). I copy my site in and the database. I access the site and it works. I never access 'install.php' and therefore, as far as I understand, the .htaccess is never created.

Does that explain it better?

@klonos
Copy link
Member

klonos commented Oct 16, 2018

It explains it perfectly @BWPanda 😉 ...now I get the security implication, but it could be solved with the following:

  • a check of whether the .htaccess file exists in docroot or not
  • a site-wide admin UI error (similar to "security updates available")
  • a warning/error in the site status page + the red notification icon in the admin toolbar
  • a link (similar to run cron/db update) that would perform the simple action of creating an .htaccess file from default.htaccess

No?

@ghost
Copy link
Author

ghost commented Oct 17, 2018

@klonos Yes, that sounds better :-)

@jenlampton jenlampton added this to the 1.12.0 milestone Oct 18, 2018
@quicksketch quicksketch modified the milestones: 1.12.0, 1.13.0 Jan 3, 2019
@jenlampton jenlampton removed this from the 1.13.0 milestone Mar 20, 2019
@jenlampton
Copy link
Member

It looks like I added a milestone to this issue back in October, but without any discussion in this issue about doing so. Does anyone remember what's left to be done, and why it was slated for 1.13? If so, can you please remind me and add the milestone candidate label? :)

@ghost
Copy link
Author

ghost commented Nov 5, 2020

I think this can be closed as "won't fix". Unless there was concensus on something...? I'll come back and close this issue in a week if not.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants