[bootstrap] Drupal 9 support

When I check this module in drupal check I found some deprecated methods that are as follows.

184/184 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

------ --------------------------------------------------------------------------
Line src/Plugin/Setting/Schemas.php
------ --------------------------------------------------------------------------
215 Call to deprecated method message() of class Drupal\bootstrap\Bootstrap:
in 8.x-3.18 and will be removed in a future release.
Use \Drupal\Core\Messenger\MessengerInterface::addMessage() instead.
------ --------------------------------------------------------------------------
------ --------------------------------------------------------------------------
Line src/Theme.php
------ --------------------------------------------------------------------------
664 Call to deprecated method message() of class Drupal\bootstrap\Bootstrap:
in 8.x-3.18 and will be removed in a future release.
Use \Drupal\Core\Messenger\MessengerInterface::addMessage() instead.
------ --------------------------------------------------------------------------

------ --------------------------------------------------------------------------
Line src/Plugin/Setting/Advanced/Cdn/CdnProviderBase.php
------ --------------------------------------------------------------------------
71 Call to deprecated method message() of class Drupal\bootstrap\Bootstrap:
in 8.x-3.18 and will be removed in a future release.
Use \Drupal\Core\Messenger\MessengerInterface::addMessage() instead.
------ --------------------------------------------------------------------------

------ --------------------------------------------------------------------------
Line src/Plugin/Setting/Schemas.php
------ --------------------------------------------------------------------------
203 Call to deprecated method message() of class Drupal\bootstrap\Bootstrap:
in 8.x-3.18 and will be removed in a future release.
Use \Drupal\Core\Messenger\MessengerInterface::addMessage() instead.
215 Call to deprecated method message() of class Drupal\bootstrap\Bootstrap:
in 8.x-3.18 and will be removed in a future release.
Use \Drupal\Core\Messenger\MessengerInterface::addMessage() instead.
------ --------------------------------------------------------------------------
------ -------------------------------------------------------------------------------------------------
Line src/Plugin/Setting/Advanced/Cdn/CdnProvider.php
------ -------------------------------------------------------------------------------------------------
221 Call to deprecated function file_prepare_directory():
in Drupal 8.7.0, will be removed before Drupal 9.0.0.
Use \Drupal\Core\File\FileSystemInterface::prepareDirectory().
227 Call to deprecated function file_unmanaged_save_data():
in Drupal 8.7.0, will be removed before Drupal 9.0.0.
Use \Drupal\Core\File\FileSystemInterface::saveData().
230 Call to deprecated function file_unmanaged_delete():
in Drupal 8.7.0, will be removed before Drupal 9.0.0.
Use \Drupal\Core\File\FileSystemInterface::delete().
------ -------------------------------------------------------------------------------------------------
------ --------------------------------------------------------------------------
Line src/Bootstrap.php
------ --------------------------------------------------------------------------
598 Call to deprecated method message() of class Drupal\bootstrap\Bootstrap:
in 8.x-3.18 and will be removed in a future release.
Use \Drupal\Core\Messenger\MessengerInterface::addMessage() instead.
------ --------------------------------------------------------------------------

I created the patch please review the patch.

Per a Slack discussion with Gábor Hojtsy regarding usage of D9 tags (Drupal 9, Drupal 9 compatibility, Drupal 9 readiness, etc.), "Drupal 9 compatibility" should be used for contributed projects that need updating and "Drupal 9" was the old tag for D8 issues before the D9 branch was ready. Doing tag cleanup here based on that discussion.

It also needs the core_compatibility clause in bootstrap.info.yml

Updated the static services with DI also updated core requirement version,
please review

is this ready to go?

thanks @Neslee Canil Pinto, and everyone else involved! I guess this will go out in the next release then?

I believe there is also a composer.json change that should be made to add the Drupal 9 support badge to the module's main page, as explained here: https://www.drupal.org/project/auto_entitylabel/issues/3111526

My info above is incorrect, apologies.

No, that is not needed, that automatic badge thing doesn't exist as explained elsewhere. No reason to change composer.json, the previous patch is fine (didn't actually test it, but I know that the latest change is not necessary).

+++ b/src/Bootstrap.php
@@ -142,6 +137,32 @@ class Bootstrap {
+  /**
+   * The messenger service.
+   *
+   * @var \Drupal\Core\Messenger\MessengerInterface
+   */
+  protected $messenger;
+
+  /**
+   * Constructs a new Bootstrap.
+   *
+   * @param \Drupal\Core\Messenger\MessengerInterface $messenger
+   *   The messenger service.
+   */
+  public function __construct(MessengerInterface $messenger) {
+    $this->messenger = $messenger;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public static function create(ContainerInterface $container) {
+    return new static(
+      $container->get('messenger')
+    );
+  }

Adding this everywhere isn't necessary. Just use the MessengerTrait.

@Berdir, thanks for the clarification. So you are saying 8 || 9 does not need to be defined in composer.yml or info.yml? How does a project get the Drupal 9 Compatible badge on the module page? I think it very useful to display this once a module is compatible so that D8 administrators can easily check all of their modules to see if they will be able to upgrade to D9. Thanks for the help.

I explained that in #3124902: Drupal 9 compatibility

Here's a patch to address the last few pieces of feedback.

A few more changes after manually installing and enabling the theme on 9.1. A few more things slipped through the cracks. But now, the theme will install and I didn't run into any further issues while browsing around on the site.

So, we have our first hard 8.8 requirement due to https://www.drupal.org/node/2966725. So, another patch that adjusts accordingly.

@heddn thanks for the patch. I have reviewed the patch and I was able to apply it successfully. All the above-mentioned review suggestions are included in the patch along with the fix of the issue's root cause.

Moving ticket forward.

Thanks.

Deprecation not fixed,

 177/177 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ ---------------------------------------------------------------------------------------------------- 
  Line   Plugin/ProcessManager.php                                                                           
 ------ ---------------------------------------------------------------------------------------------------- 
  81     Call to deprecated method getInlineElementTypes() of class Drupal\bootstrap\Plugin\ProcessManager:  
         in bootstrap:8.x-3.21 and is removed from bootstrap:8.x-4.0.                                        
         This method will be removed when process managers can be sub-classed.                               
 ------ ---------------------------------------------------------------------------------------------------- 

 ------ ----------------------------------------------------------------------------------------------- 
  Line   Plugin/Provider/ProviderBase.php                                                               
 ------ ----------------------------------------------------------------------------------------------- 
  212    Call to deprecated method getAssets() of class Drupal\bootstrap\Plugin\Provider\ProviderBase:  
         in 8.x-3.18, will be removed in a future release.                                              
 ------ ----------------------------------------------------------------------------------------------- 

 ------ ------------------------------------------------------------------------------------------------------------ 
  Line   Plugin/ProviderManager.php                                                                                  
 ------ ------------------------------------------------------------------------------------------------------------ 
  70     Call to deprecated method processDefinition() of class Drupal\bootstrap\Plugin\Provider\ProviderInterface:  
         in 8.x-3.18, will be removed in a future release. There is no                                               
         replacement for this functionality.                                                                         
 ------ ------------------------------------------------------------------------------------------------------------ 

 ------ ------------------------------------------------------------------------------------------------- 
  Line   Plugin/Setting/Advanced/Cdn/CdnProvider.php                                                      
 ------ ------------------------------------------------------------------------------------------------- 
  263    Call to deprecated method getApi() of class Drupal\bootstrap\Plugin\Provider\ProviderInterface:  
         in 8.x-3.18, will be removed in a future release. There is no                                    
         replacement for this functionality.                                                              
 ------ ------------------------------------------------------------------------------------------------- 

                                                                                                                        
 [ERROR] Found 4 errors                                                                                                 
                                                                                                                        

1. As per the added comment #13

this new method should also be marked as internal and deprecated, linking to this issue since it will be removed once this issue can be resolved

Call to deprecated method getInlineElementTypes() of class Drupal\bootstrap\Plugin\ProcessManager. This is not resolved yet.

2. Call to deprecated method getAssets(), processDefinition(), getApi() will be removed in future releases but currently there is no replacement for these functionalities so keeping them like they are.

Patch #19 is applied successfully.

Other than above-mentioned deprecations all others are fixed including the changes recommended in #12

$ ./vendor/bin/drupal-check -d themes/bootstrap/src/Bootstrap.php 
 1/1 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%


                                                                                                                        
 [OK] No errors   

$ ./vendor/bin/drupal-check -d themes/bootstrap/src/Plugin/Setting/Schemas.php
 1/1 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%


                                                                                                                        
 [OK] No errors                                                                                                         
                                                                                                                        

$ ./vendor/bin/drupal-check -d themes/bootstrap/src/Theme.php
 1/1 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%


                                                                                                                        
 [OK] No errors                                                                                                         

$ ./vendor/bin/drupal-check -d themes/bootstrap/src/Plugin/Setting/Advanced/Cdn/CdnProviderBase.php
 1/1 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%


                                                                                                                        
 [OK] No errors                                                                                                         

./vendor/bin/drupal-check -d themes/bootstrap/src/Plugin/Setting/Schemas.php
 1/1 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%


                                                                                                                        
 [OK] No errors                                                                                                         

These are all not drupal 9 deprecations but something within the module, it is not in scope for this issue.

awesome, thanks @sja112!

Just tested this on Drupal 9.0-beta2. RTBC++

is any further work or reviewing needed for this to be committed?

1. +++ b/bootstrap.info.yml
   @@ -1,4 +1,4 @@
   -core: 8.x
   +core_version_requirement: ^8.8 || ^9
   

   Really not a fan of this (would prefer to support all of 8.x and 9.x)... but I guess it can't be helped due to \Drupal\Core\Security\TrustedCallbackInterface which is now a requirement in 9.x.

2. +++ b/src/Plugin/Setting/Advanced/Cdn/CdnProvider.php
   @@ -218,16 +219,16 @@ class CdnProvider extends CdnProviderBase {
   -      file_prepare_directory($provider_path, FILE_CREATE_DIRECTORY | FILE_MODIFY_PERMISSIONS);
   +      \Drupal::service('file_system')->prepareDirectory($provider_path, FileSystemInterface::CREATE_DIRECTORY | FileSystemInterface::MODIFY_PERMISSIONS);
   ...
   -        file_unmanaged_save_data($import_data, $file, FILE_EXISTS_REPLACE);
   +        \Drupal::service('file_system')->saveData($import_data, $file, FileSystemInterface::EXISTS_REPLACE);
   ...
   -        file_unmanaged_delete($file);
   +        \Drupal::service('file_system')->delete($file);
   

   Anytime \Drupal::service('') is used, it should be assigned to a standalone variable with an inline typehint above it pointing to the service's interface. Also, no point in retrieving the same service several times in the same method.

3. +++ b/src/Plugin/Setting/Schemas.php
   @@ -193,6 +196,8 @@ class Schemas extends SettingBase {
   +    $messenger_class = new static();
   +
   
   @@ -200,7 +205,7 @@ class Schemas extends SettingBase {
   -      Bootstrap::message(new FormattableMarkup('@message' . $list->renderPlain(), [
   +      $messenger_class->messenger()->addMessage(new FormattableMarkup('@message' . $list->renderPlain(), [
   

   If this is because it's in a static method, then it should probably just use \Drupal::messenger().

@markcarver, can you just provide an updated patch with those changes?

Working on feedback from #27.

I applied 3096963-30.patch and the only remaining errors are internal bootstrap deprecations, which are outside the scope of this issue:

$ ./vendor/bin/drupal-check themes/bootstrap/
 185/185 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ --------------------------------------------------------------------------------------------------- 
  Line   src/Plugin/Markdown/AllowedHtml/Bootstrap.php                                                      
 ------ --------------------------------------------------------------------------------------------------- 
         Class Drupal\markdown\Plugin\Markdown\AllowedHtmlInterface not found and could not be autoloaded.  
  18     Class Drupal\markdown\Plugin\Markdown\AllowedHtmlInterface not found and could not be autoloaded.  
 ------ --------------------------------------------------------------------------------------------------- 

 ------ ---------------------------------------------------------------------------------------------------- 
  Line   src/Plugin/ProcessManager.php                                                                       
 ------ ---------------------------------------------------------------------------------------------------- 
  81     Call to deprecated method getInlineElementTypes() of class Drupal\bootstrap\Plugin\ProcessManager:  
         in bootstrap:8.x-3.21 and is removed from bootstrap:8.x-4.0.                                        
         This method will be removed when process managers can be sub-classed.                               
 ------ ---------------------------------------------------------------------------------------------------- 

 ------ ----------------------------------------------------------------------------------------------- 
  Line   src/Plugin/Provider/ProviderBase.php                                                           
 ------ ----------------------------------------------------------------------------------------------- 
  212    Call to deprecated method getAssets() of class Drupal\bootstrap\Plugin\Provider\ProviderBase:  
         in 8.x-3.18, will be removed in a future release.                                              
 ------ ----------------------------------------------------------------------------------------------- 

 ------ ------------------------------------------------------------------------------------------------------------ 
  Line   src/Plugin/ProviderManager.php                                                                              
 ------ ------------------------------------------------------------------------------------------------------------ 
  70     Call to deprecated method processDefinition() of class Drupal\bootstrap\Plugin\Provider\ProviderInterface:  
         in 8.x-3.18, will be removed in a future release. There is no                                               
         replacement for this functionality.                                                                         
 ------ ------------------------------------------------------------------------------------------------------------ 

 ------ ------------------------------------------------------------------------------------------------- 
  Line   src/Plugin/Setting/Advanced/Cdn/CdnProvider.php                                                  
 ------ ------------------------------------------------------------------------------------------------- 
  265    Call to deprecated method getApi() of class Drupal\bootstrap\Plugin\Provider\ProviderInterface:  
         in 8.x-3.18, will be removed in a future release. There is no                                    
         replacement for this functionality.                                                              
 ------ ------------------------------------------------------------------------------------------------- 

                                                                                                                        
 [ERROR] Found 6 errors

maybe @jcnventura or someone else can tests against Drupal 9.0-beta2 again...

Looks like the interdiff.txt in #30 is reversed (e.g. it shows withdrawing MessengerTrait while the new patch actually adds it, as should be), so better ignore the interdiff. Patch looks good otherwise.

@fgm, the interdiff is fine. It's an interdiff of #19 to #30 and contains the changes I asked for in #27.

---

So... D9 is supposed to release today, how is it June already?! Sigh.

As the 8.x-4.x branch is nowhere near ready for a stable release (and considering that this project is technically not following semver, yet), I suppose we're going to have to backport this to 8.x-3.x and create a release there.

Maybe to semi-get around this I'll bump the minor so it's 8.x-3.30 (maybe 8.x-3.90?), so people can see there's a significant version bump and will get curious enough to read the release notes/project page (which I'll also add a note on).

Idk, there's really no elegant solution here.

Are you just concerned about the 8.8 requirement or are there also other changes that could actually break something?

I think most users are used to the fact now that modules require 8.8 and they have to be careful. All my big projects do: token, redirect, pathauto, paragraph.... Got a bunch of support requests at the beginning, but has been quiet recently. And the project and release page now show the core compatibility.

From a technical standpoint, I've been backporting most commits that have been made to 8.x-4.x into 8.x-3.x as well, mostly because there haven't been that many differences yet.

I'm more concerned with arbitrarily requiring a new major dependency which introduces new APIs that didn't exist before. This means that someone cannot simply upgrade the base theme, they will also have to upgrade core (which they may or may not be ready for).

The other consideration I have to make is the reality that this isn't a module, it's a [base] theme.

I know I probably harp on that distinction a lot, but it's typically for a very good reason: modules can do things that themes can't.

The least of which has been #474684: Allow themes to declare dependencies on modules which still isn't even technically released (or rather will be released today alongside D9).

So we can't even really effectively add a soft dependency for that matter.

---

I've actually been fiddling around with BC support on my work with #3142418: Support multiple libraries per plugin.

Essentially, it copies core's interfaces and then aliases them when they don't exist.

I think that might be a better approach than setting such a hard requirement here.

Let me extract a portion of that patch into a separate issue and then see what I can't come up with here.

edit: #3145331: [BcSupport] Extend real core classes/interfaces if they exist and alias them when they don't

I'm more concerned with arbitrarily requiring a new major dependency which introduces new APIs that didn't exist before. This means that someone cannot simply upgrade the base theme, they will also have to upgrade core (which they may or may not be ready for).

@markcarver, is the statement above referring only to users running drupal core 8.7.x and earlier?

Correct. A project that increases major dependencies like what is being proposed here means that package managers like Composer will automatically upgrade to the latest release (for contrib), even if their core is technically locked to a much lower version than is suddenly required by said contrib project.

Suddenly there's a "conflict". Either at the package manager level (which can leave confusing messages) or in fatal errors/WSOD on Drupal's side.

This is how/why people get frustrated with package managers. It's not because they don't work, but because project maintainers don't actually follow proper semver (or whatever convention they're supposed to) and suddenly they bump major dependencies in the middle of a minor release, which is what this technically would be (8.x-3.MINOR).

And before anyone says "Oh just have them upgrade their core", really hasn't spent any time on a large project or a project that is ultimately governed by 3rd party APIs/integrations/clients/stakeholders. The reality is sometimes that isn't an immediate possibility.

That's why this whole "oh you can support both Drupal 8 and Drupal 9" push that's going around really only makes sense if

a) the project is relatively simple or only used APIs that haven't changed at all in both timespans or

b) was only recently created and doesn't have to deal with a boatload of legacy code it was originally built on and still has to support... because otherwise, what's the point of versions in the first place?

It's just frustrating that there's this push and I get that people want it, but there's a responsibility that maintainers have (or rather should have) to adhere to when it comes to releasing stuff like this. It also doesn't help that our development/release cycles aren't even on the same planetary time scale as core, nor have the same amount of developers to work on this stuff.

Which is why comments like #28 are a little insulting. Sometimes the best we can do is offer a review as a way to push the development forward. We don't always have the ability to spin up our locals, completely shift gears and suddenly do what we can see from just reading the patch. Sometimes we're working on other d.o projects (that also want the same level of attention) or client work (that actually requires our attention/pays the bills).

I actually do care about this project, have spent most of my career building and maintaining it.

This issue will be resolved ASAP, I just don't know when that will be.

I will attempt to get to it later today (no promises).

p.s. I'm sorry for the rant and it wasn't really directed at anyone specific. Just needed to get a few things off my chest is all.

> Correct. A project that increases major dependencies like what is being proposed here means that package managers like Composer will automatically upgrade to the latest release (for contrib), even if their core is technically locked to a much lower version than is suddenly required by said contrib project.

No, it will actually not do that. composer.json will contain a drupal/core ^8.8 || ^9 requirement after this commit and composer will then simply not update to this version if you run composer update. It would only affect people who manually update or use the web UI to do so (which in 8.8 now also includes checks for this).

Increasing required versions on your dependencies is perfectly valid in minor releases (which technically every contrib release is that doesn't use semver), and it is what the whole support Drupal 8 and 9 movement is about. It's not support *any* Drupal 8 version. It's support Drupal 8.something and Drupal 9, where "something" depends on the project.

Anything older than 8.8 is, as of today, officially unsupported. Anyone using such a version will not receive security updates anymore and is absolutely, 100% on their own. Personally, I'm not interested in spending any time* on accounting for those sites in my projects, but its your decision as a maintainer on what to do here. Mostly just wanted to point out that this is a non-issue for composer-based sites which handle that just fine as I wrote.

* Yes I know, I literally just spent time on writing this comment. Ignore that :p

composer will then simply not update to this version if you run composer update

Exactly the point I am making. It leaves people who have to stay on an older core version unable to update a contrib project that suddenly makes this kind of change. This also means that any security release for the contrib project that may pop up will either a) require manual patching or b) left with insecure code. Again, just another area where contrib is on a different time scale from core.

Anything older than 8.8 is, as of today, officially unsupported. Anyone using such a version will not receive security updates anymore and is absolutely, 100% on their own.

https://www.drupal.org/project/usage/bootstrap

There are still more D7 installations of this project than there are D8.

Even though I have, for the most part, effectively brought that major/branch pretty close to EOL, it doesn't change the fact that people "need" contrib in a different way than they do core.

Core's LTS !== contrib LTS

Increasing required versions on your dependencies is perfectly valid in minor releases

Technically, this is true. Practically, it's a nightmare. Again, this is why a lot of people are left with a bad taste in their mouth when it comes to package managers. It's not the package manager's fault, it's the project for bumping dependencies without regard to the larger dependency chain/ecosystem it's actually installed alongside. Or the history of how it was built in the first place. Simply put: it's an unnecessary burden; one that can easily be avoided if you treat major dependency upgrades as your own major bump.

Which is why comments like #28 are a little insulting.

Sorry, that's my fault. We depply appreciate the work you are doing here, and I didn't mean to come off as snide but see how it can appear that way,

> There are still more D7 installations of this project than there are D8.

That's not what I meant, I'm only talking about Drupal 8. Drupal 7 still has security support until Nov 2021 and will have extended support after that. D7 is not relevant to this discussion.

And yes, if you need to do a security update after this, then those people are left out, but again, they are at that point already most likely running insecure core versions or have them patched and then they can patch contrib too.

These projects that you're talking about already can't update token, pathauto, redirect, paragraphs, webform, ... bootstrap is just one more in a long list of projects.

To be clear, I'm not arguing against you, it's your decision on how much time you want to invest to support older core versions. I'm only trying to give you reasons why it is IMHO OK to require 8.8 now and not worry about older core versions.

No worries @xeM8VfDh. I didn't mean that maliciously. I only pointed it out to preface the sentences following it. We all have a lot on our plates and sometimes a review, or in this case, a quick comment is what we're can muster at the moment :D Typing thoughts on a topic is sometimes easier than working on the actual code implementation. It can even lead to new ideas on how to solve it like realized in #37.

That's not what I meant, I'm only talking about Drupal 8. Drupal 7 still has security support until Nov 2021 and will have extended support after that. D7 is not relevant to this discussion.

It kind of is. If only for the context for which it brings: contrib is different; each project is different.

It's an unfortunate reality of this project, but it comes with a lot of baggage. Simply slapping on some new property and removing deprecated code isn't going to "magically make things better". In fact, it would probably just make things a lot more confusing and create even more issues in this queue.

These projects that you're talking about already can't update token, pathauto, redirect, paragraphs, webform, ... bootstrap is just one more in a long list of projects.

Modules, which can explicitly state their dependencies. This is new territory for themes. Besides, I've never been one to jump off a bridge because others do.

And yes, if you need to do a security update after this, then those people are left out, but again, they are at that point already most likely running insecure core versions or have them patched and then they can patch contrib too.

Leaving people out of a security update regarding my project because I chose to update a major dependency in my minor which forces a specific dependency version seems... backwards and almost like a hostage situation.

And yes, if they're patching core, they're probably patching other stuff too. Doesn't mean we have to add to that pain if it could technically be avoided in the first place.

To be clear, I'm not arguing against you, it's your decision on how much time you want to invest to support older core versions.

I understand. I guess I just value DX and the upgrade process a little differently. I personally like when things "just work" when you're working in the same "major" of whatever it is. Rather than having to explain why you have to suddenly update your dependency just to receive some minor bug fixes or security update.

To be fair, if I had known that core was going to treat its minors more like majors over the years, I probably would have taken a much different approach with this project and avoided the whole "bootstrap major version parity" in favor of newer major branches over time.

Alas, semver was new for core, and now contrib has it too. Things have changed drastically since the "D8" version of this project was initially released and there have been many lessons learned; c'est la vie.

Ok. This adds some relatively minor BC layers that should allow the 8.x-3.x branch to continue working in D8 and now also in D9.

No bigger than previous patches, but also no interdiff since this is a completely different approach.

This looks good with 8.9.0 but I have a problem if anyone can please help; when I try to upgrade to Drupal 9 I get:
drupal/bootstrap dev-3.x requires drupal/core ~8.0 -> satisfiable by drupal/core[...] (Listing all the 8.x versions)
I can't figure it out, and starting to wonder if that's just meant to happen because it's determining that with the module before the patch or something.

That's because this issue hasn't been committed to the codebase yet. The base theme has to be manually downloaded, patched and installed for D9 currently.

I just manually tested #45 with both a D8 and D9 install.

I found a couple of issues that I'm fixing here.

Going to tentatively mark this as RTBC. Would like it someone would confirm this before I commit and push a release.

thanks @markcarver for the dedication and commitment to keeping users (even those on older versions) covered! Much appreciated!

On a completely unrelated note, I wonder when in the hell DO is going to start supporting user @ mentions and comment references *sigh*

Yeah... that has a complicated history lol

#448074: @ style mentions for usernames which send notifications for D.o
#2416625: Add At.js autocomplete in textareas on d.o

I wasn't aware of those issues, thanks for pointing me in the right direction

+++ b/src/BcSupport/TrustedCallbackInterface.php
@@ -0,0 +1,27 @@
+
+if (!interface_exists('\Drupal\Core\Security\TrustedCallbackInterface')) {
+  /* @noinspection PhpIgnoredClassAliasDeclaration */
+  class_alias('\Drupal\bootstrap\BcSupport\BcAliasedInterface', '\Drupal\Core\Security\TrustedCallbackInterface');
+}

address did it like this:

-// @codingStandardsIgnoreStart
-// @todo remove this BC layer once support for Drupal 8.7 is sunsetted
-if (interface_exists('\Drupal\Core\Security\TrustedCallbackInterface')) {
-  interface TrustedCallbackInterface extends \Drupal\Core\Security\TrustedCallbackInterface {}
-}
-else {
-  interface TrustedCallbackInterface {}
-}
-// @codingStandardsIgnoreStop

then you define the interface inline and don't have to create that other one. might need to copy that into each class that needs it on the other side, but I think it's just one or two?

I'd rather use class_alias and create an empty shell to inherit from rather than using @codingStandardsIgnoreStart, something I adamantly try to avoid when possible. It's also just a little cleaner and more logically appropriate to use class_alias, which was designed for use-cases like this.

Some comments

1. +++ b/src/Bootstrap.php
   @@ -623,6 +630,23 @@ class Bootstrap {
   +   * @deprecated in bootstrap:8.x-3.22 and is removed from bootstrap:5.0.0.
   +   *   Use mb_strpos() instead.
   +   * @see https://www.drupal.org/project/bootstrap/issues/3096963
   +   */
   +  public static function fileSystem() {
   

   Nit: Copy/pasta. Comment is a little off here on what is happening.

2. +++ b/src/Utility/Unicode.php
   @@ -188,4 +188,79 @@ class Unicode extends CoreUnicode {
   +  public static function substr($text, $start, $length = NULL) {
   +    @trigger_error('\Drupal\bootstrap\Utility\Unicode::substr() is deprecated in bootstrap:8.x-3.22 and will be removed before bootstrap:5.0.0. Use mb_substr() instead. See https://www.drupal.org/project/bootstrap/issues/3096963.', E_USER_DEPRECATED);
   +    return mb_substr($text, $start, $length);
   

   Question, even though core switched from unicode to mb_substr as of a certain version and deprecated the legacy approach, does that mean we still need to use the legacy approach for as long as we can? Couldn't we just use mb_substr directly since it is available from php itself?

3. +++ b/src/BcSupport/TrustedCallbackInterface.php
   @@ -0,0 +1,27 @@
   +  /* @noinspection PhpIgnoredClassAliasDeclaration */
   

   Is this needed? Seems specific to phpstorm.

#54.1: Great catch :D

#54.2: I was actually thinking about this. In theory, it shouldn't matter (symfony/polyfill-mbstring has always been apart of D8). In reality, it should actually be implementing the full code from previous core versions (just in case): https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Component%21Util...

#54.3: Considering that I (and many others) exclusively use PHPStorm and use it to run tons of inspections, it helps, yes.

My bootstrap-based site seems to run just fine with #56 installed on Drupal 9.0.

Code-wise, I think we're good now. I didn't get a chance to test on D9, but I don't see anything in here that raises alarm. +1 on RTBC from me.

With this version of the patch, upgrade status still mentions use of obsolete libraries:

• The 'drupal.vertical-tabs' library is depending on a deprecated library. The "core/matchmedia" asset library is deprecated in drupal:8.8.0 and is removed from drupal:9.0.0. See https://www.drupal.org/node/3086653
• The 'image_widget_crop/cropper.integration' library is not defined because the defining extension is not installed. Cannot decide if it is deprecated or not.

The second one seems to be related to the image_widget_crop module, not to the version included in this theme ? And I didn't see a reference to matchmedia in the discussion yet. Is another upstream fix needed for this one to work ?

Great catch on core/matchmedia @fgm!

The second just extends the image_widget_crop module to provide integration with it when using the module in a Bootstrap based theme; it's not a deprecation, just missing.

Back to RTBC? I'd already tested the rest of this patch on a D9 site. And hadn't really seen any issues w/ not having that library. But I also wasn't inspecting for js errors.

Thanks @markcarver - It's working fine for me now (Was getting errors with #45).

I uninstalled Bootstrap from Core 8.9.0 then applied patch, reinstalled: all fine.
Uninstalled Bootstrap, upgraded Core with Composer to 9.0.0, added bootstrap 3.x-dev and applied this patch (manually :)), installed Bootstrap and my sub-theme, all fine.

Including one small change to explicitly require drupal/core in composer.json. I know the d.o's packaging adds this via the composer facade, but it's just nice to keep this kind of info with the project itself.

edit: interdiff includes changes from other recent issue commits

Released: 8.x-3.22

This was technically a feature request.