🇺🇸United States @mikelutz

smustgrave → credited mikelutz → .

gábor hojtsy → credited mikelutz → .

smustgrave → credited mikelutz → .

I don't see that in the latest code for paragraphs module:

> interface ParagraphInterface extends ContentEntityInterface, EntityOwnerInterface, EntityNeedsSaveInterface, EntityPublishedInterface {

I just noticed that the actual methods of EntityOwnerInterface are deprecated on the Paragraph entity, but I don't know the whole story or for how long that's been the case.

https://git.drupalcode.org/project/paragraphs/-/blob/8.x-1.x/src/Entity/...

I'm not sure what exactly we could do here, except carve out an exception in core for paragraphs. Two issues here, first, under the current code, the paragraph owner is the owner of the parent entity. Your issue isn't that the paragraph is owned by anonymous, it's that you haven't given it a parent yet at the time you are trying to validate it. Secondly, it seems that paragraphs has deprecated their use of EntityOwnerInterface, meaning in the near future, even if you did assign the parent during the migration (which is not something you would typically do) It's still going to not provide an owner to validate against though the normal entity api. You would have to know it was a paragraph and that paragraphs validate in the UI through the parent's owner, then go find that (possibly through nested paragraphs) specially for paragraphs, which we won't do in core.

Without paragraphs providing an owner to validate against, there isn't much we can do, since we can't really validate, because we don't know what permissions to validate against for things like text format of a body field.

I think this might have to be paragraph's problem. I don't see how we can support validation in this instance if paragraphs is going to do something funky outside the entity api in it's own validation.

I am open to suggestions though.

This is neither reasonably possible, nor desired. The map table is a record of what we did, not something that reflects the current state of the system. More importantly the meaning of destination ids is completely plugin defined. They have no relationship to entities at all, so there would be no way to connect a deleted entity to a map table entry. The issue summary describes a custom workflow and problem that is best solved in custom code, where the custom code is aware of the specific relationships between migration destination maps and entities in use in that particular site.

I just want to note that "wrap" is just syntactical sugar for

plugin: get
      source:
        - ~

and unwrap is just syntactical sugar for

plugin: extract
      index:
        - 0

With maybe some single_value, multiple_value plugins in there if you need them, though if the purpose is to pipe a value into a handle_multiples plugin that can't handle a single value, there will often not be much point, as those plugins don't care if the value is a multiple or not.

(BTW, I'm starting to come around to just documenting the 'hack' with the get plugin above and officially supporting it. I've found enough cases where it's useful)

benjifisher → credited mikelutz → .

hestenet → credited mikelutz → .

hestenet → credited mikelutz → .

mikelutz → changed the visibility of the branch 3463800-allow-batchsize-to to hidden.

mikelutz → created an issue.

mikelutz → created an issue.

mikelutz → created an issue.

mikelutz → created an issue.

For lack of my 7am brain being able to come up with a clever way to do this in bash, running

<?php
$f = fopen("5278.diff", "r");
$regxp = "/https:\/\/([a-z0-9A-Z\.-]+)[^a-z0-9A-Z\.-]/i";
$matches = [];
$results = [];
while (!feof($f)) {
  $line = fgets($f);
  if (preg_match($regxp, $line, $matches)) {
    $results[$matches[1]] = isset($results[$matches[1]]) ? $results[$matches[1]] + 1 : 1;
  }

}
array_multisort($results, SORT_DESC);
foreach ($results as $name => $count) {
  echo $name . ": " . $count . "\n";
}

on the original MR gives

example.com: 1412
www.w3.org: 747
www.example.com: 136
jsonapi.org: 77
jqueryui.com: 58
jquery.org: 48
www.drupal.org: 40
api.jqueryui.com: 37
drupal.org: 22
www.test.com: 20
wikipedia.org: 18
google.com: 14
docs.jquery.com: 9
www.doctrine-project.org: 8
fonts.fontprovider.com: 8
php.net: 7
www.sqlite.org: 7
en.wikipedia.org: 7
api.drupal.org: 7
www.collegehumor.com: 7
symfony.com: 6

along with ~80 additional domains with < 5 usages

The numbers are a bit off from the actual work between codebase changes and the vendor changes to be undone. Also the xmnls in the svg files should not be changed, https://www.w3.org/2000/svg is an identifier, not a url, and it should not be https. Still probably a good baseline to start with. Having discussed scoping child issues of metas like this several times with @xjm in the recent past, I'm fairly confident we should be doing this by domain for the top domains and then catch the one-offs in a separate issues or issues as it makes sense. These are probably not novice issues (at least not all) as there are many places where this should not be changed at all. Let's start with a child issues for
example.com: 1412
www.w3.org: 747
www.example.com: 136
jsonapi.org: 77
jqueryui.com: 58
jquery.org: 48
www.drupal.org: 40
api.jqueryui.com: 37
drupal.org: 22
www.test.com: 20
<\code>

and go from there. We can also probably standardize on replacing http://example.com with https://www.example.com and http://drupal.org with https://www.drupal.org while we are at it.

That's an option, but let's hold back. Even with reduced scope, this can't go in before 11.1, as we are already in RC for 11.0 and this is unlikely to be backported to the 11.0 series as it's a new feature and not a bug fix. If we can't make progress on the parent issue in the next couple of months we can revisit the reduced scope here to try to get it into 11.1, but in general, I like the scope and functionality, the timing is just bad with the other API changes we are working on.

AFAICT you can fall back version_compare() unless BOTH left & right are 11.x and 11.0.x (though not sure what order they come in, might have to check both orders).

Plus, doing this, would just cause the problem to come around again when we open 11.1.x in a few weeks.

Not the decision maker but just going to re-iterate, I think this is a bad idea. The hacks I see for uninstalled modules seem like just the tip of the iceberg. What about removed modules. removed libraries. uninstalled themed. profiles.... it seems like a loosing battle for little benefit.

I'm not fully seeing the issue here. What do you mean about removed modules, removed libraries, etc? If you uninstall/remove code providing a constant/enum used in your active configurations, you will have problems, as you should. The 'hack' here only adds a module to the classloader as it's config is read in preparation to install it. This seems appropriate to me, as the config you are installing should be able to use enums defined in the module you are installing. This isn't about loading enums and consts from uninstalled modules, this is to load enums and consts from a module that we are in the process of installing. I really think the inline comments are scarier than what the code is doing, and I made a couple suggestions on the comments. I also added a test that I think shows another issue, config referencing constants that are in the modules .module file. I think we will have to include the .module file before parsing the config as well.

Also, I tried to find a more elegant way to prevent double caching of the autoloading storage, but I really couldn't, so I just added a second instanceof check in the Comparer. I don't like it.

I'm going to move it to a full fledged constraint in Core/Validation to solve the dependency issue. To make it work for both snippets and full pages, I think we can have the constraint just wrap snippets in "" .. "". We could either define whether to do so in an argument to the constraint, or try to autodetect, but I'm going with a constraint argument that defaults to snippet because the schema would want to decide which it allows. The fast_404 html here would want a full page and want to reject a snippet so I don't see autodetecting being practical.

(Ironically, I was resurrecting 📌 Create a trait and base class to implement \Drupal\Component\Plugin\ConfigurableInterface Needs review the other day and I realized the we nearly did the same intercomponent dependency in the patch we committed and reverted all those years ago. We were going to call Component\Utility\NestedArray from Component\Plugin and didn't notice)

Okay, I reviewed this pretty thoroughly as it was being built and gave it a final once over. As far as I can tell, we've covered all edge cases, tests look good, documentation looks good. Let's ship it.

Postponing this on 📌 Allow process plugins to flag a row to be skipped Needs review , as there are a number of things in here that don't make sense in the context of the changes we are trying to make to skipping rows, such as doing away with using Exceptions to do so.

Alright, I think I've resurrected this from the ashes. New merge request on 11.x, fixed issues in #123and #132

123.1 Mention the interface instead of the trait in ConfigurablePluginBase.

I mention both, since the base class uses the trait anyway.

123.2 In ConfigurableTrait, document that the $configuration property is also defined \Drupal\Component\Plugin\PluginBase.

done

123.3 Decide on if ConfigurableTraitTest needs adjustments.

It did, though opposite from the previous suggestions of moving all tests to getMockForTrait, as getMockForTrait is getting deprecated in phpunit, so I converted all tests to use the same concrete test class that uses the trait.

123.4 Make sure that the changes in base classes for certain plugin types are mentioned in the release notes.

I added the list to the release notes snippet and the CR

123.5 (optional) Check if setConfiguration() still needs to be overridden in WorkflowTypeBase.

It's an issue of overriding it and keeping tests, or resetting it to the standard and adjusting all the tests to handle the reordered returned array. I also removed use of the trait entirely from ConditionPluginBase for the same reasons, updating it to use mergeDeepArray broke a few tests and will require adjusting the order of items in umami config to pass tests (as they would be exported in a different order, causing a test fail) It's better to explore converting both of those pluginbases to use mergeDeepArray in a followup, if at all, where we can review the test changes and potential BC issues more thoroughly in isolation from the trait and base class work being provided here.

123.6 Rebase the merge request.

done.

132 > implement \Drupal\Component\Plugin\ConfigurableTrait directly

Is that the right wording? Does one 'implement a trait'? I'd have said 'use' or 'import'.

I agree, switched the wording to use `use`.

In addition, I added SelectionPluginBase to the plugin bases that we convert (it was added after the original MR was created here), and removed ConditionPluginBase for reasons described above.

I moved ConfigurableTrait from Component/Plugin to Core/Plugin. In the original patch, we didn't notice that having the trait in Component meant we added a undocumented dependency on Component/Utility to Component/Plugin, and we shouldn't do that. The interface can live in the Component, the trait can live in Core next to the base class (which was already there because that needed to extend Core's PluginBase and not Component's. Despite the fact that the interface lives in Component, it makes more sense to have the implementations live in Core.

I updated the Change Record (and unpublished it, as it was never unpublished after we reverted years ago.

I fixed a few coding standards/phpstan/phpunit10 issues that have changed things over the years.

I think this should be committable now, or close to it hopefully.

cross posting from 📌 Move source_module and destination_module from Migrate to Migrate Drupal Needs work

I think we need to rethink this, and 📌 Move source_module and destination_module from Migrate to Migrate Drupal Needs work in light of 📌 [Policy] Migrate Drupal and Migrate Drupal UI after Drupal 7 EOL Fixed

source_module is a migrate_drupal thing, as this issue points out, but despite the work here, the source_module key is still a thing defined in migrate as part of the source plugin annotations, and 3421014 is moving it over to the attributes defined in migrate. I'm working out the order of operations here, but I think the concept of source_module as a source plugin attribute/annotation key needs to be deprecated in Drupal 11 along with everything else migrate_drupal.

I have not followed the raw details on the annotation to attribute conversion for plugins, and I'm not sure what would break if we did this, but my gut says that here, we should NOT support source_module as a source plugin attribute, nor should we convert the d7 source plugins that we are deprecating in D11 over to attributes. I'm sure there's some core test somewhere that ensures core plugins which support attributes use them, and we may have to find a way around that for the sources we are deprecating, but I feel like it's worth it, and it saves us from having to deprecate source_module as it can just go away with annotations.

I think we need to rethink this, and 📌 Convert MigrateSource plugin discovery to attributes Needs work in light of 📌 [Policy] Migrate Drupal and Migrate Drupal UI after Drupal 7 EOL Fixed

source_module is a migrate_drupal thing, as this issue points out, but despite the work here, the source_module key is still a thing defined in migrate as part of the source plugin annotations, and 3421014 is moving it over to the attributes defined in migrate. I'm working out the order of operations here, but I think the concept of source_module as a source plugin attribute/annotation key needs to be deprecated in Drupal 11 along with everything else migrate_drupal.

I have not followed the raw details on the annotation to attribute conversion for plugins, and I'm not sure what would break if we did this, but my gut says that in 3421014, we should NOT support source_module as a source plugin attribute, nor should we convert the d7 source plugins that we are deprecating in D11 over to attributes. I'm sure there's some core test somewhere that ensures core plugins which support attributes use them, and we may have to find a way around that for the sources we are deprecating, but I feel like it's worth it, and it saves us from having to deprecate source_module as it can just go away with annotations.

mikelutz → changed the visibility of the branch 2852463-create-a-trait to hidden.

mikelutz → changed the visibility of the branch 9.2.x to hidden.

mikelutz → changed the visibility of the branch 2852463-configurable-plugin-trait-10.1 to hidden.

mikelutz → changed the visibility of the branch 11.x to hidden.

mikelutz → changed the visibility of the branch 11.x to hidden.

Closing this, as we just use ConfigurableInterface now.

mikelutz → created an issue.

NW for @quietone's requests. I left a few comments with some thoughts for folks to think about, but none are hard requirements for this MR, they could be done here if they make sense, or they could all be debated and done in a follow-up (or not)

Oops, yes, thank you.

PP on 📌 Add PHP_INT_MAX to integer schema configuration Active

I don't want to block this, I just wanted to point it out and ask.

Right, that was my question. We now get an AutoloadingStorage here. Previously we got a FileStorage, and since FileStorage has it's own static cache, we were not setting a storage cache. Now we have an AutoloadingStorage, which is based on FileStorage and uses it behind the scenes, but we are setting the storage cache as well, since it's not a FileStorage. I was wondering if we needed to. As I was stepping through HEAD and the patch trying to find the difference that caused the bug in listAll it was one of the differences I noticed, as the importer was operating on a CachedStorage object in the patch versus a FileStorage object on HEAD. It wasn't the problem, but I'm wondering if we need the sourceCacheStorage with the Autoloading storage, or if we should carve out the same exception for AutoloadingStorage here that we have carved out for FileStorage.

@deprecated in drupal:11.1.0 and is removed from drupal:11.1.0. Inject

Should this not be removed in 12.0.0?

https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co.... Found this too, while I was debugging the tests? Do we need to add the Autoloading storage here?

in the Storage Comparer:

  if ($source_storage instanceof FileStorage) {
      // FileStorage has its own static cache so that multiple reads of the
      // same raw configuration object are not costly.
      $this->sourceCacheStorage = new NullBackend('storage_comparer');
      $this->sourceStorage = $source_storage;
    }

Lol, took forever to find the listAll() bug, and you fixed it before I could push :-)

catch → credited mikelutz → .

mikelutz → made their first commit to this issue’s fork.

This one seemed like fun.

I preserved BC, so if orderby is null, or === 'key', or === 'value' existing behavior is preserved.

if orderby is a string other than 'key' or 'value' it is treated as a key inside the value. I did not require a prefix 'subkey:' as it seemed unnecessary. If you need to reference a key named 'key' or 'value' it can be done using the advanced syntax that follows. To drill down into deeper items in the sequence arrays, separate keys with a '/' (see tests). This mimics the format we use to access nested array items in the migration system.

if orderby is an array, each item is assumed to be a sort directive, and the sequence is sorted by the first, then the second, and so on. The sequence is finally sorted by the element values (while preserving keys) so as to be deterministic.

Each sort directive can be either a string, which is taken to be an element key as decribed above, or mapping containing at minimum a 'sort_by' key (containing the element key described above) and optionally 'sort_order' and 'sort_flags' keys.

sort_order and sort_flags accept as strings the constant names described here: https://www.php.net/manual/en/function.array-multisort.php sort_flags can either be a string containing a single flag, or a sequence containing multiple flags to OR together.

It would be nice if 📌 Allow parsing and writing PHP constants and enums in YAML files Needs work were in, and we could avoid parsing the constants as strings altogether. If we don't wait, we should probably add some error checking here to ensure only the expected strings are used.

A dozenish tests are provided to test combinations of sort orders, ensure that string keys are preserved. and to demonstrate capibilities.

A new `UnsupportedSequenceSortConfigException` is provided and thrown if a sequence element does not contain a key that the schema requests that we sort on.

mikelutz → made their first commit to this issue’s fork.

fjgarlin → credited mikelutz → .

This can be committed with review. The distinction mentioned in #19 is the distinction beteween the plugin setting the value to NULL vs the Executable setting the value to NULL when the SkipProcess Exception was thrown. Personally I don't think there is a distinction because throwing a SkipProcessException set the value to NULL, so I feel like the plugin was responsible for the NULL anyway. But the point is moot, as ✨ Allow process plugins to stop further processing on a pipeline RTBC refactored away the exception and now the process plugin does literally set the value to NULL. I consider the doc change trivial, but it's fine to commit, as it does improve the docs.

mikelutz → changed the visibility of the branch 3336070-skiponempty-is-ignored to active.

mikelutz → changed the visibility of the branch 3336070-skiponempty-is-ignored to hidden.

Also switching this to major based on "The major priority is used for issues that are not critical, but that do have significant impact or are important by community consensus." As recipes are a hot item right now I would consider having the example recipe be functional for 10.3.0 would be important, but feel free to reduce if I am wrong.

Crediting @Josue2591 as he wrote the code. I just opened a new MR against core, cherry picked the commits and (hopefully correctly) fixed some merge conflicts.

Alright, lets try that one.

mikelutz → changed the visibility of the branch distributions_recipes-3447994-3447994-example-recipe-isnt-with-test to hidden.

Posting a relevant slack thread.

Core contrib question: this issue 🐛 Example recipe isn't functional Needs review got moved into Needs Work status by a bot. Is there something we can do to get it back to RBTC?

Participants:

Related Issues:

See https://onlinephp.io/c/faedc

Try catch is like 10x slower than doing most any kind of type checks or conversions. Because explode ends up having to do the same conversions anyway PLUS run the unneeded explode call, and then check the error handler and find the catch block. It's always going to be more efficient to just do our own type checks.

Admittedly removing the type check to the constructor negates the real world impact of this as we are no longer checking the configuration on every row, but a microsecond is a microsecond..

I'd prefer not to use a try-catch here, it's certainly not needed, as we know precisely what explode will accept, a string that is not the empty string. Ultimately I would like to require the configuration item to be a string. It's easy enough to force a string in yml by quoting it. So lets set ourselves up to be strict. Go with this

if (!is_string($configuration['delimiter'])) {
  trigger_error('Using a non-string as a delimiter in the explode plugin is deprecated in 11.1.0 and will throw a MigrateException in 12.0.0. Ensure your value is quoted in your migration yaml if necessary', E_USER_DEPRECATED);
  if (is_array($configuration['delimiter'])) {
    $configuration['delimiter'] = '';
  }
  $configuration['delimiter'] = (string) $configuration['delimiter'];
}
if ($configuration['delimiter'] === '') {
  throw new \Drupal\migrate\MigrateException('Delimiter muse be a non-empty string value.....')
}
parent::__construct($configuration, $plugin_id, $plugin_definition);

Move it to the constructor, since this is an error on the configuration and not the value, keep the MigrateException for now. I'm tempted to build something new with annotations and validators, but no time for that now..

The correct check is `if (!is_string($x) || $x==='') throw.. `

mikelutz → created an issue.

Adding related issues

I'm not certain that we can do anything here, but we definitely can't do this. All of these sources being moved are scheduled to be deprecated and removed completely by d12 along with the migrate_drupal module, so we can't set a deprecation message telling people to use the migrate_drupal versions before d12 because those aren't going to exist anymore. I think at best we could introduce a copy of the trait to migrate_drupal and switch the sources over to it without doing any deprecation notices about moving things, and I'm not sure we should bother doing that at this point either.

Reverted the form changes and moved them to a follow-up 📌 Improve UX of system performance form max-age form input Active to determine how best to improve the form UX

Also reverted the changes to ConstraintManager that exposed DivisibleBy as we are no longer going to use it here.

Thanks to @catch and @phenaproxima for the slack discussion. Back to NR

mikelutz → created an issue.

The latest push makes the change I recommended above, constraining the value to an integer between 1 and 3156000 and reconfiguring the form to use a number field with the same constraints. I didn't see any specific tests around this form to adjust, but there might be a functional Javascript test somewhere that will need to be adjusted.

Done.

Fixed coding standards and rebased to see if the validation checker can resolve updated dependencies.

Yes, that brings drush back into play, do you think it makes an argument to using: `update_skip_future_update`?

I'm not sure what you mean here, I may be missing something. All that function can do is log that we should not run that future update when we eventually see it. If the thing that calls that future update at some point in the future doesn't know to check the logs and see it shouldn't be run, then it's going to be run. There isn't going to be anything we could do in the present to prevent an update from being run in the future without the cooperation of the thing that is calling the future update function, unless, as I noted in the MR, we don't check that from the runner and instead wrap the contents of the future update with some sort of `if (should_be_skipped(__FUNCTION__)` every time.

The only issue I see with this is you need to know if you're going to backport, I think this solution is meant to solve for the case where you didn't know you were going to backport when you wrote the original update hook.

That's not really an issue, as all the code for this goes only in the backport, at least as things stand until we figure out what to do about drush.

Regarding #72, that feels like a huge potential issue to require skipping a certain minor version depending where you update from.

In theory, no. In reality, it could be, but with the scheduled support cadence, there is really no way around it. Going from 10.4 or 10.5 to 11.0 would effectively be a downgrade. I'm leaning towards recommending my agency keep clients on 10.3 until we are ready to execute upgrades to 11. My mind could change depending on what happens in the next 6 months, but I'm thinking the sweet spot for upgrades to 11 will be to go from 10.3 to 11.0 in the first half of 2025 followed by a quick update to 11.1, and then 11.2 in the second half of 2025.

Regarding #73 wouldn't the update_skip_future_update function handle drush too?

Well, looks like we bypassed the update_skip_future_update function in favor of just calling \Drupal::service('update.update_hook_registry')->skipFutureUpdate($module, $number); directly in the module, but either way, no. That method just logs that the future update should be skipped, but it's the update runner that currently is responsible for checking that state and deciding not to run the future update when it eventually sees it. But drush has a separate batch runner to actually execute the hooks, which at the moment doesn't know it needs to check to see if an update should be skipped before running it, so if you were running drush updb as everything sits now, you would end up re-running the future update hook. Admittedly update hooks often are or can be made idempotent, (if thing A exists, convert it to thiing B. On rerun, thing A doesn't exist so the hook does nothing anyway). That would further reduce the times we would need to use this (i.e. only if the hook needs backporting to 10.x and only if thinks would break if it were to be re-run)

I left some comments on the first approach wondering how to deal with the drush runner, which has its own version of updateDoOne that would need to be updated as well, and wondering if we could go back to attributes since we are changing updateDoOne. We could keep most of the code the same, but use #[UpdateSkip('11101', '11.1')] on the backported hook and look for it in UpdateDoOne and call the service to save the skip data right after or before we run the hook. It might be cleaner that way.

+1 on the RTBC

The solution does not specifically mention the process plugins and destination plugins in core modules outside of migrate.

I think we can make some of these decisions as we do the deprecations. I think the destinations can stay where they are for the most part. They are appropriate plugins for the remaining migrate API, and are useful for anyone using migrate to import data from any source. The process plugins should be looked at on a case by case basis, but if they are generally useful and specific to the module in question then they are fine to stay. If they are very specific to converting something from a d7 format to a d8+ format, they can be deprecated along with the migrate_drupal modules. (I think most of the process plugins that aren't in migrate core will fall into this category, but there may be a few worth keeping)

I tweaked the wording about destinations a bit to indicated that module specific destinations can stay where they are and added a note about deprecating field and process plugins outside of migrate_drupal that are migrate_drupal specific. Leaving RTBC as I think this is just clarifying what we've already decided, and obviously if we deprecate migrate_drupal and it's migrate field plugin manager we would have to deprecate the related plugins throughout core too.

No, but we've never had a major version receiving bugfixes while the next major's first minor was down to security support or out of support entirely, so we've never needed to.

For me this is a big disadvantage - you can't tell people what to upgrade to.

Update 11000 is added to 11.1.x, it's backported to 10.4.x as 10401. Someone updates to 10.4.0, then updates from there to 11.0.0, they've now gone 'backwards' to a release where that update never existed. We need to prevent this, but can do so by checking if there are skipped logged updates that were run in 10.4.x, that don't exist at all in the release we're updating to, and also are higher than hook_update_last_removed() - in that case we'd show an updates requirement error and refuse to update, telling people to update to a later minor release instead.

I started coding an attribute and storage to be able to give an actual version number and I realized we may be completely over thinking this in both approaches here by trying to calculate if we can do an upgrade or not based on missing update hooks. We don't need to. We already know that if you are on 10.3 you can upgrade to 11.0, if you are on 10.4 you have to go straight to 11.1, and if you are on 10.5 you have to go straight to 11.2.

We are adding code and work to hunt for maybe a situation where possibly if you are on an early enough patch version of 10.4 or don't have any modules installed that may have a database update hook skipped in 11.0 that we can technically let you upgrade to only 11.0, but why bother? 11.0 will be in security phase by then, 11.1 will be the recommended version and more importantly regardless of database schema, by the first patch release, 11.4 will have bug fixes in it that aren't in 11.0. Regardless of whether the upgrade to 11.0 is technically possible at that point, we would be allowing them to upgrade to a more buggy version which would be a bad experience we want people to avoid anyway.

I think we just need to document and require that 10.4 must upgrade to 11.1, and 10.5 must upgrade to 11.2, enforce that in system_requirements, and not even mess with trying to calculate allowable upgrade versions via finding missing update hooks. We could address that in a separate issue, and it wouldn't even be a blocker until 10.4 beta.

mikelutz → changed the visibility of the branch 3437129- to hidden.

I thought I would take a stab at the other option, just to see what it looked like. Not tested yet, and likely horribly broken. I'm happy to keep working on it if there is interest, and happy to close it if we are going with #33, but I wanted to at least attempt to put some code where my mouth is, since I was advocating for it.

Besides the potential for testing to reveal an inherant flaw in the plan, there is still a bunch of cleanup to do. update_get_update_function_list should get deprecated and moved into UpdateHookRegistry, update_resolve_dependencies should also probably be deprecated and moved to refactor away from passing $starting_update around, things like that.