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

Add webflo/drupal-core-strict #477

Open
aleksip opened this issue Mar 13, 2019 · 14 comments

Comments

@aleksip
Copy link

commented Mar 13, 2019

Considering how much trouble the recent Twig 1.38.x issues seem to have caused, should webflo/drupal-core-strict be added? I always add it to my drupal-composer/drupal-project-based projects.

@aleksip

This comment has been minimized.

Copy link
Author

commented Apr 4, 2019

Looks like there is a new issue (#480 and https://www.drupal.org/project/drupal/issues/3045349) that would have been mitigated by using webflo/drupal-core-strict.

Seems like a reasonable idea to default to using core dependencies that have been tested? This is probably something that most people want most of the time?

@cilefen

This comment has been minimized.

Copy link

commented Apr 29, 2019

@greg-1-anderson

This comment has been minimized.

Copy link
Collaborator

commented Jun 13, 2019

See #264. I still am of the mind that most people need drupal-core-strict, and most people do not need drupal-core-require-dev most of the time, and would rather see the former here than the later if we had to pick just one.

It would be neat of Composer would actually have features to support these kinds of projects directly. The current model makes composer update drupal/core --with-dependencies more difficult, as you must also mention the related parallel projects your site is using.

@aleksip

This comment has been minimized.

Copy link
Author

commented Jun 13, 2019

I guess the importance of DX depends on the intended users/use cases for this project? My impression is that drupal-composer/drupal-project is the most popular starting point for Composer based regular Drupal installations, and that many (if not most) users are site builders who might not be too familiar with Composer.

@greg-1-anderson

This comment has been minimized.

Copy link
Collaborator

commented Jun 13, 2019

As for the issue of DX, users must learn one new trick: composer update drupal/core webflo/drupal-core-strict --with-dependencies. Put that in the project README.

I think the benefits of protecting inexperienced users from being the first to discover bugs in dependencies is a good tradeoff.

@mxr576

This comment has been minimized.

Copy link
Contributor

commented Jul 17, 2019

webflo/drupal-core-strict and webflo/drupal-core-require-dev would not exist if drupal/core would be highest-lowest tested by the maintainers. Related issue: https://www.drupal.org/project/drupal/issues/2947893

drupal/drupal is a composer project that could have and has composer.lock committed which specifies the expected version from the dependencies.
drupal/core is a composer library that should not have and do not have composer.lock committed. Consequently the version constraints inside its composer.json specifies a version range of the compatible dependencies. Unfortunately these version ranges are outdated/not validated with highest-lowest testing and therefore issues like you referenced above or like this: https://www.drupal.org/project/drupal/issues/2965887 could happen when drupal/core is installed.

These webflo packages are only providing a workaround by "imitating" composer project behavior on a composer library.

@greg-1-anderson

This comment has been minimized.

Copy link
Collaborator

commented Jul 17, 2019

Lowest testing would solve issues such as https://www.drupal.org/project/drupal/issues/2965887.

I disagree with the premise that highest testing would obviate the need for drupal-core-strict, though. It has happened a number of times that some dependency of drupal/core has released a new version that had some bug or issue that expressed itself in Drupal. These issues would be detected, but not prevented by highest testing. Historically, these bugs have been detected and fixed very quickly. They still cause a lot of disruption for a lot of people; drupal-core-strict avoids this disruption.

I assert that drupal/core is neither a project nor a library in the Composer sense of the word. It is something else that is not supported by Composer. See concept of parent projects in reposer.

@mxr576

This comment has been minimized.

Copy link
Contributor

commented Jul 19, 2019

It has happened a number of times that some dependency of drupal/core has released a new version that had some bug or issue that expressed itself in Drupal.

Yes, I am very well aware of these issues, especially the Twig ones, but I believe those only affected new installations and they could be revealed before they would break production sites if developers performed proper testing before prod deploys. The reason is simple, in case of existing projects where the composer.lock is in VCS, always the "latest stable and tested dependencies" gets installed when a new environment is created. So even if a Drupal core dependency introduces a breaking change or a bug, it could be found before updating the installed versions in a project's composer.lock.

They still cause a lot of disruption for a lot of people; drupal-core-strict avoids this disruption.

Yes, it could avoid this problem, although I would rather call it "hiding". If you install drupal/core + webflo/drupal-core-strict you always get exactly the same version just like if composer.lock would be committed. But it is a worse solution then composer.lock because webflo/drupal-core-strict's composer.sjon also contains specific versions instead of version ranges. Why? Because it prevents installations of newer package versions. If a developer would like to install a new dependency - let it be a Drupal module or a PHP library - that requires a newer version from a dependency which still matches the version range then what is it in drupal/core's composer.json but it is higher then it is in webflo/drupal-core-strict's composer.json then the installation of the new library fails with a dependency error. This is a bad DX. As a deveper, I cannot install a package that should be compatible with the set of dependencies that I installed (including drupal/core) but I cannot.
I also wonder how webflo/drupal-core-strict will support when both Symfony 3 and 4 will be supported by core. There will be specific builds for "validated" Symfony 3 and 4 versions?

I would rather aim for a less drastic and more practical solution to eliminate or precisely identify those Drupal core dependency updates that could break Drupal sites. This could be achieved with the highest testing. If a highest test would run on Drupal core in every hour for example and it would always install the latest available versions from all packages then breaking changes could be identified almost immediately and developers could be notified on various channels that something needs to be fixed and which dependency version should be installed until the fix is available.

@greg-1-anderson

This comment has been minimized.

Copy link
Collaborator

commented Jul 19, 2019

I believe those only affected new installations

This is not the case. Sites not running drupal-core-strict have been broken by running composer update commands that bring in a newer core dependency than was tested with the currently released version of Drupal core.

breaking changes could be identified almost immediately and developers could be notified

Historically, when breaking changes like this happen today, they are identified on the same day, and core developers are aware right away. However, it takes more than an hour to make a Drupal release. I think last time it was a day or two. That's a decent window in which users not using drupal-core-strict can become confused and/or inconvenienced.

While I agree that highest testing provides value (c.f. g1a/composer-test-scenarios), it is not a substitute for drupal-core-strict.

@WarpedOne

This comment has been minimized.

Copy link

commented Jul 30, 2019

Some of the problems caused by Twig were fixed in later versions. I believe the Symfony problem was caused by a change that affected how Drupal was incorrectly using Symfony. Part of the issue is when old versions of Drupal are used to create dev sites and versions aren't locked, or creating a new site/updating current site with only composer update. A partial fix could be adding a change date onto the webflo/drupal-core-require-dev (i.e. 8.7.3.20190701) that would allow adding “twig/twig”: “^1.38.0” or a specific version to conflict area. It would cause more maintenance, especially if it goes back many versions, but would cut down drastically on all the time spent running down solutions that have already been found. Just look in slack for days after an issue appears, people are still posting and responding to issues that can appear differently to different situations. Google will not always match up errors. For a distribution, I am using drupal-core-strict. But there are many others for many reasons that will not. (Steps off soap box) Sorry, just putting in my 2cents worth, hoping to improve things. Happy that webflo exists. Maybe leading to a better (automated?) solution.

@greg-1-anderson

This comment has been minimized.

Copy link
Collaborator

commented Jul 30, 2019

It is likely that Drupal 8.8.0 will support Composer-managed Drupal sites with (and without) a relocated document root, and it is also likely that once this is supported, some equivalent to webflo/drupal-core-strict will be provided. Given that, perhaps it is no longer necessary to introduce that project here.

@aleksip

This comment has been minimized.

Copy link
Author

commented Aug 7, 2019

The official Drupal documentation on Composer states that "the recommended approach (especially for beginners to Composer) is to use the composer template at drupal-composer/drupal-project". (Emphasis mine.)

Based on comments here drupal-composer/drupal-project seems to be targeted for experienced developers. I don't consider myself a beginner but I'm no Composer expert either, and some of the discussion here goes way over my head. It also doesn't seem to be relevant to what I use drupal-composer/drupal-project for.

Hopefully the new official way will be better for beginners who just want to install and update a regular Drupal site. However, until it is available and widely adopted, I think a lot of beginners to Composer will continue to use this project.

@WarpedOne

This comment has been minimized.

Copy link

commented Aug 16, 2019

Checking the webflow/drupal-core-require-dev 8.7.6 composer.json requires a fixed drupal version.
But webflo/drupal-core-strict 8.7.6 composer.json doesn't list drupal/core.

If both are fixed, couldn't the version in the drupal-core/drupal-project for webflo be allowed to float and let composer find the correct webflo for the drupal version specified in the base composer.json?

That would allow a webflo/drupal-core-strict:^8.0.0 to be a dependency that wouldn't need to be specified, but "composer update drupal/core --with-dependencies" would work for either strict or require-dev?

And could the drupal-composer/drupal-project list dev dependencies, but not install them unless dev was overridden?

As far as webflo/drupal-core-require-dev allowing updates to versions with problems, it would be easy to have a release date added to the require-dev (i.e. 8.7.6.20190816) to allow for the version to be updated to add a conflict "twig/twig": "^1.40.0" or "twig/twig": "1.40.0" depending on whether it is a change they fixed in the next release, or just showing up an error in drupal code, like symfony/http-foundation did.

The reason for all this is that people need to go back to older versions for various reasons, or can't update yet. I've seen people raising issues in slack and posting multiple issues on d.o whenever these problems come up, even after they are fixed.

Maybe auto-updates and composer-in-core will fix all this, but we need to look at better ways of using composer.

@greg-1-anderson

This comment has been minimized.

Copy link
Collaborator

commented Aug 16, 2019

I think it's a bug that webflo/drupal-core-strict does not have a restriction on drupal/core. Without it, composer update drupal/core --with-dependencies has no requirement to update drupal-core-strict, which means that you could end up keeping your dependencies locked to an older version than your current drupal version has in its composer.lock file.

The best solution to this is to have webflo/drupal-core-strict require drupal/core: self.version, and have Drupal sites require webflo/drupal-core-strict, but not drupal/core. In this instance, you could get a new version of drupal/core via composer update webflo/drupal-core-strict --with-dependencies, and your pinned libraries and core directory would always be in alignment. Doing this would be a backwards-compatibility-breaking change to webflo/drupal-core-strict, but it could be done for a similar project with a different name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.