- Issue created by @joachim
- π«π·France dqd London | N.Y.C | Paris | Hamburg | Berlin
is this possibly a reason for this behavior?
@trigger_error('Calling InfoParserDynamic::__construct() without the $app_root argument is deprecated in drupal:10.1.0 and will be required in drupal:11.0.0. See https://www.drupal.org/node/3293709', E_USER_DEPRECATED);
- π¬π§United Kingdom joachim
This is the bit that needs to be changed:
// Determine if the extension is compatible with the current version of // Drupal core. try { $parsed_info['core_incompatible'] = !Semver::satisfies(\Drupal::VERSION, $parsed_info['core_version_requirement']); }
- π«π·France dqd London | N.Y.C | Paris | Hamburg | Berlin
Oops, sorry for the tagging, it was actually just a "looked at" the tags, but should not be added. Pressed enter too quick at night and I was actually too tired to look at it.
- π¬π§United Kingdom joachim
The tags look right to me - accidental removal because of a cross-post.
- π«π·France dqd London | N.Y.C | Paris | Hamburg | Berlin
Hah! double *facepalm* here....
- Merge request !5036Added leniency for non-core extension core compatibility detection when core... β (Open) created by joachim
34:30 46:50 Running- Status changed to Needs review
about 1 year ago 1:04pm 18 October 2023 26:10 12:05 Running- Status changed to Needs work
about 1 year ago 1:48pm 18 October 2023 - π¬π§United Kingdom longwave UK
This has broken some legitimate tests.
- π¬π§United Kingdom joachim
Hmmm any thoughts on how we allow for this:
> Behat\Mink\Exception\ResponseTextException: The text "System core incompatible semver test (incompatible with this version of Drupal core)" was not found anywhere in the text of the current page.
- last update
about 1 year ago 30,357 pass, 7 fail - last update
about 1 year ago 30,417 pass - Status changed to Needs review
about 1 year ago 8:13am 20 October 2023 - Status changed to Needs work
about 1 year ago 8:48am 20 October 2023 - last update
about 1 year ago 30,357 pass, 9 fail - Status changed to Needs review
about 1 year ago 8:23am 21 October 2023 - Status changed to Needs work
about 1 year ago 10:42am 21 October 2023 - π¬π§United Kingdom joachim
@longwave the new failures with the current condition mean that all tests that do this sort of thing:
$base_info = ['name' => $extension_name]; if ($extension_type === 'theme') { $base_info['base theme'] = FALSE; }
must now set the package.
Is that ok?
- πΊπΈUnited States cmlara
Iβll note there is an open issue to consider removing core_version_requirement in light of the fact core is moving to a composer only workflow. β¨ Remove requirement for core_version_requirement for module to be compatible with Drupal 9 Active
Perhaps that is a better alternative?
Iβve made the mistake a few times of failing to update the version requirements in a module, CI rightfully threw and error warning me that if I were to tag a release it would not work. Iβve also hade CI alert me that a dependency was not ready.
There are various testing regiments Iβve seen, some test against latest release and others test against latest head.
This change appears to make it more likely that a module owner may accidentally create a release that can not actually be installed unless they make sure to only test against tagged core releases.
Iβm not a big fan of development branch code having special logic, it makes it harder to know the real issues, and makes it more likely to confuse users.
- π¬π§United Kingdom joachim
β¨ Remove requirement for core_version_requirement for module to be compatible with Drupal 9 Active has been open for 2 years and is not moving forward very quickly.
This issue is something that needs to be fixed **soon** so that contrib developers can work on their projects against the core development branch.
- πΊπΈUnited States cmlara
has been open for 2 years and is not moving forward very quickly.
True though there hasnβt been a good argument for it either. This might be what is needed to revive it. Iβm sure that issue stalled out a bit as effort went into adopting modules for D9 and D10 as its need became much less important with the community stepping up. This is now not a technical reason rather than a contributor time reason.
This issue is something that needs to be fixed **soon** so that contrib developers can work on their projects against the core development branch.
Isnβt this more related to the fact that until we move to a MAIN branch we have 11.x defined as
const VERSION = '11.0-dev';
, If anything we would wanting that to be 10.2-dev until we split off 10.2 to its own branch and than it gets updated to 10.3-dev. IIRC that was because some tests were dependent upon the branch name matching the version, not sure if thatβs been solved yet, but that would be a root issue here wouldnβt it?That also I believe brings us back to we need to lab test a few solutions to the GIT Branch Alias problems we have encounter as IIRC thatβs one of the major problems preventing switching to a MAIN branch. Last I heard that was just waiting for manual testing of a few scenarios to unblock other development work.
To me, and maybe there something Iβm missing, this feels like this is just ignoring the root problems which are already known and that if effort was put into them, make this fix moot at best, and at worst make it a DX/UX trap.
Also Iβll add that Drupals own development docs recommend developing Contrib against the next Core release until itβs in beta as API is subject to change. I donβt usually expend any effort until at least the alphas and itβs usually a one-off test just to make sure there are no major breaks when Iβll look at it in beta as I expect most of core to be in heavy flux until that time.
Certain modules (like Devel) do need to develop against the latest core early to be useful, however that is a fraction of the ecosystem and workarounds likely exist (patching the module locally, or locally disabling the core check, running a Main branch that doesnβt publish dev releases that all code goes into before being cherry-picked to the latest active release branch, etc)
- π¬π§United Kingdom joachim
> Certain modules (like Devel) do need to develop against the latest core early to be useful, however that is a fraction of the ecosystem and workarounds likely exist (patching the module locally
It's not just that.
As a developer, working on Drupal core, I need Devel module for DX. And also Admin Toolbar, and other contrib modules that make core more usable.
Thinking about it, fixing β¨ Remove requirement for core_version_requirement for module to be compatible with Drupal 9 Active won't solve this anyway -- the extension system would be changed to read the composer.json file, which will ALSO report that the module can't be installed on core 11.x.
> Isnβt this more related to the fact that until we move to a MAIN branch we have 11.x defined as const VERSION = '11.0-dev';,
Yes. But AFAIK, we don't know how this sort of thing will work once we have a 'main' branch. It may be we need a similar hack to this MR.
- πΊπΈUnited States cmlara
won't solve this anyway -- the extension system would be changed to read the composer.json file, which will ALSO report that the module can't be installed on core 11.x
That is also true with the suggested fix in this issue, composer itself will still reject the install. The core_version_requirement key is included in the package server during publication (as it should be). In both cases a developer would still need to do "require drupal:^11@dev as 10.3.0" (or similar) to bypass composer or downloading the modules via source manually.
As a developer, working on Drupal core, I need Devel module for DX. And also Admin Toolbar, and other contrib modules that make core more usable.
That is more an argument that those modules should maintain a development branches with a composer.json that does indicate support for whatever the next release is in the main branch equivalent (but don't publish it as a tagged release so it requires explicit effort to download as a source branch with composer or git)
Yes. But AFAIK, we don't know how this sort of thing will work once we have a 'main' branch. It may be we need a similar hack to this MR.
True, though I would contend we should figure that out first, that could allow this to be more surgical or even unnecessary.
- π¬π§United Kingdom joachim
> That is also true with the suggested fix in this issue, composer itself will still reject the install
There's already something to handle that in https://github.com/joachim-n/drupal-core-development-project
And I don't think it will necessarily be true -- a core system that checks for core compatibility in module composer.json files might not go via the Composer API but might read the JSON files directly.
> That is more an argument that those modules should maintain a development branches with a composer.json that does indicate support for whatever the next release is in the main branch equivalent (but don't publish it as a tagged release so it requires explicit effort to download as a source branch with composer or git)
No! I REALLY don't want to be running dev branches of the tools I need to work on core. I want stable versions that I can rely on.
> True, though I would contend we should figure that out first, that could allow this to be more surgical or even unnecessary.
I asked core managers about this at DrupalCon Lille and they said that a. There is no timescale at all for when we move to a main branch, and b. There is no sort of plan yet on how that will work.
I think we need contrib projects to be installable on core 11 now.
- πΊπΈUnited States cmlara
There's already something to handle that in https://github.com/joachim-n/drupal-core-development-project
Could that project also patch the info.yml than too?
I think we need contrib projects to be installable on core 11 now.
That is already doable with local patching (once you have the source downloaded) using standard composer tools, without making Drupal respond differently on development branches.
Core 11 doesn't even exist yet.
What I think you really want is for D.O to open a 10.3 branch now, still commit (and tag all issues) to D11 (so we have a 'main' branch) but backport them in real time to 10.3 so testing can work correctly.
- π¬π§United Kingdom catch
We branched 10.2.x recently and it's identical to 11.x in all respects except the version name.
The next branches will be 10.3.x, at that point 11.x is likely to start accepting 11.x-only changes.
Given that, this is unlikely to be an issue until we actually have a 'main' branch, and that should be solvable via a branch alias (we hope). All this to say I'm not sure this issue is actually needed, the past six months has mostly been a temporary transitional situation until 11.x and 11.0.x coexist.
- π¬π§United Kingdom longwave UK
I still think we're going to need this, or something similar, when we have the
main
branch? Otherwise nobody will be able to install any contrib there, because nothing should declare compatibility with it. So in the meantime while11.x
is acting asmain
I think we should still try to fix it. - π¬π§United Kingdom joachim
https://www.drupal.org/project/devel_a11y β is another reason for needing this.
@longwave This is pending a decision from you about #14.
- π¬π§United Kingdom longwave UK
Re #14 I think it's fine for modules that are testing functionality outside of this to have to set the "Testing" package. I am not sure what alternative we have apart from trying to special case them in the
if
statement instead, which feels more fragile. - Status changed to Needs review
9 months ago 4:03pm 27 February 2024 - Status changed to Needs work
9 months ago 8:58pm 27 February 2024 - πΊπΈUnited States smustgrave
Did not test but appears to have some test failures.
- Status changed to Needs review
9 months ago 11:23am 28 February 2024 - πΊπΈUnited States smustgrave
There a good way to test?
Applied the MR and tried to download a module but still get the error.
- πΊπΈUnited States smustgrave
- drupal/devel[5.1.0, ..., 5.x-dev] require drupal/core ^9 || ^10 -> found drupal/core[9.0.0-alpha1, ..., 9.5.x-dev, 10.0.0-alpha1, ..., 10.3.x-dev] but the package is fixed to 11.x-dev (lock file version) by a partial update and that version does not match. Make sure you list it as an argument for the update command.
- π¬π§United Kingdom joachim
That's a Composer error. This issue is not about Composer. It assumes you have already installed the package with Composer and you are now trying to install it in Drupal.
(To install it with Composer, use https://github.com/joachim-n/drupal-core-development-project)
- πΊπΈUnited States smustgrave
So I cloned the devel module with the MR still applied and still can't install.
- π¬π§United Kingdom joachim
Have you cleared caches? It turns out the extension system caches .info file data -- see the InfoParser class.
- π¬π§United Kingdom joachim
Can you try using the admin UI rather than Drush please?
I just managed to enable admin toolbar using that.
- πΊπΈUnited States smustgrave
Ah okay I found the issue after using admin_toolbar which did work.
Devel has php: 7.4 in it's info files which blocked it, even though the the warning is
Requires: Drupal Core (^9 || ^10) (incompatible with version 11.0-dev)
But that's probably a separate issueAnyway using admin_toolbar did work with the MR. So +1 from me.
- Status changed to RTBC
9 months ago 9:47pm 3 March 2024 - πΊπΈUnited States smustgrave
Been a few days and I rechecked with another module and didn't have any issue.
- π©π°Denmark ressa Copenhagen
The title makes this issue sound 11.x specific ... wouldn't this be more precise?
"Allow any contrib extensions to be enabled in dev-version of Drupal core"
- π¬π§United Kingdom joachim
> "Allow any contrib extensions to be enabled in dev-version of Drupal core"
AFAIK it's only a problem on 11.x and in the future main branch.
If you're developing on 10.2.x, then contrib modules that say ^10 will work because they satisfy the semver requirement.
- π©π°Denmark ressa Copenhagen
Ok, I just assumed that this MR will also override
core_version_requirement: ^11
in a contrib extension, and allow installing it in a future D12-dev. - π¬π§United Kingdom joachim
That's true, it will. But I'd assumed that by then core will be on a `main` branch?
- Status changed to Needs work
8 months ago 3:25pm 14 March 2024 - π³πΏNew Zealand quietone
I removed the code changes to InfoParserDynamic and ran InfoParserUnitTest.php which passed. Therefore I think this should have another test case. And I was wondering what happens for a contrib module that has a test module in the Testing package.
- π¬π§United Kingdom joachim
> And I was wondering what happens for a contrib module that has a test module in the Testing package.
Urgh yeah, will they fail to install in tests?
How are contrib modules currently testing against 11.x?
- πΊπΈUnited States smustgrave
Can say currently I am not testing against 11.x
IF I really really need to though I clone the module and edit the info file to let me install. Super pain though for a module that has a bunch of dependencies.
- π¬π§United Kingdom joachim
We're going to need another hack :(
'Core Testing' and 'Testing'?
Or a special property for those info files that specifically need to opt-out of this because they are testing compatibility?
- πΊπΈUnited States cmlara
How are contrib modules currently testing against 11.x?
Most are probably not.
For those that are testing in GitLabCi the 'opt into next major' job which re-writes info.yml files to allow installing on D11. A patch file can be provided to patch the dependencies into compatibility and the Lenient plugin allows downloading dependencies with composer.
For local: The same process one would use to handle modules that currently do not support D10, Composer Lenient and patches (which is the same as for GitlabCi)
As noted previously I still suggest against development release specific feature in the core and contend β¨ Remove requirement for core_version_requirement for module to be compatible with Drupal 9 Active would be a better route for this.
https://www.drupal.org/project/backward_compatibility β was mentioned in β¨ Allow Drupal core developers and site builders to download contrib module in the next version Active though I'm not sure how it interacts with kernel tests, presumably functional tests would be covered if used and unit tests don't care. It is certainly an option for solving the argument in #20 to allow installing development tools to work on core.
- π¬π§United Kingdom joachim
I don't understand how β¨ Remove requirement for core_version_requirement for module to be compatible with Drupal 9 Active would help here, since its issue summary says:
> If the core_version_requirement line is present in a project, process it as normal.
β¨ Allow Drupal core developers and site builders to download contrib module in the next version Active is not relevant here -- that issue is about installing packages with Composer. This issue is about enabling modules with the Core extension system.
- π©π°Denmark ressa Copenhagen
Actually, β¨ Allow Drupal core developers and site builders to download contrib module in the next version Active is both about downloading and installing, which I have clarified in the title.
There's also this suggestion by @mglaman:
It sounds like the
core_version_requirement
in the module should just be deprecated in favor of composer.json constraints, period. - πΊπΈUnited States cmlara
If the core_version_requirement line is present in a project, process it as normal.
Since that issue hasn't been finalized it could be enhanced to remove honoring the core_version_requirement at all. Assuming that part of the proposal wasn't changed, over time it would likely be adopted by contrib to remove the line to reduce maintenance burden (personally I'm good with just composer.json and find it more flexible). This would likely be especially true for modules like Devel that know they need to be used on bleeding edge releases.
is not relevant here -- that issue is about installing packages with Composer. This issue is about enabling modules with the Core extension system.
I was simply giving credit to an issue that references this issue and reminds us that the backwards_compatibility module (that I forgot existed) was created as an opt-in method for contrib to solve many (if not all?) of the pain points previously raised here without forcing core to adopt dev-branch only logic.
We are now also saying this is "going to need another hack". In my experience when a developer finds themself describing a solution as a 'hack' it has been a sign development is either going down a less ideal path, or the code is trying to do something very brittle. It is not always the case, however in my experience it is a warning sign to take a step back and consider a different approach.
- π¬π§United Kingdom joachim
> It sounds like the core_version_requirement in the module should just be deprecated in favor of composer.json constraints, period.
That won't work either. Or rather, it'll still need a hack here, because a contrib module that says it's compatible with '^10' won't work with core 11.
And yes, it's a hack. But so is composer-lenient! :)
- πΊπΈUnited States cmlara
That won't work either. Or rather, it'll still need a hack here, because a contrib module that says it's compatible with '^10' won't work with core 11.
I donβt believe it would need a hack here if the info compatibility check was deprecated in 10.3 and completely ignored in 11.x.
This issue would not fix the composer side either. Downloading with composer is never mandatory, I can always obtain the files myself and put it in my code base locally and happily satisfy composer by informing it that my local project provides the files.
- π¬π§United Kingdom joachim
The composer side is already fixed.
If you deprecate then remove the info compatibility check, and don't necessarily install with Composer, then how is the extension system supposed to determine if a module is compatible or not?
- πΊπΈUnited States cmlara
If you deprecate then remove the info compatibility check, and don't necessarily install with Composer, then how is the extension system supposed to determine if a module is compatible or not?
The simple answer is that it does not attempt to. A site owner has "forced" the software in, at that point it is on the site owner to take responsibility for where they utilize it.
- π¬π§United Kingdom joachim
Ah right. I think that kind of change is a much bigger conversation than what this issue is trying to do, and will take much longer to be dealt with (personally, I'm REALLY not in favour of replacing a human-readable and -writeable and commentable format with JSON which is at best only one of those things).
- π¬π§United Kingdom longwave UK
I am not sure we can drop
core_version_requirement
entirely until we have stopped shipping tarballs. Once everyone is on Composer it seems fine to rely on Composer's constraints, but until then users are free to download incompatible tarballs and that is the only mechanism to prevent them from being used. But that is for another issue anyway.I was wondering what happens for a contrib module that has a test module in the Testing package.
If this is no different now to how it currently stands then I think that is out of scope for fixing in this issue; this patch solves an immediate problem and while that is also a problem, it is a slightly different one that I think can be handled in a followup?
- πΊπΈUnited States cmlara
My main request is that this not be "dev branch specific". My tests should run the same against a tagged stable as the dev branch.
Can the logic at least be changed to a Settings parameter to enable this?
- if (str_ends_with(\Drupal::VERSION, '-dev')) { + if (Settings::get('ALLOW_MODULE__INSTALL_BYPASSS_VERSION_CHECKS', FALSE)) {
A settings is not dev banch specific. is explicitly opt-in requiring the 'site owner' to make a conscious decision to utilize it.
Being opt-in allows me as a developer to test against upcoming mainline strictly and yet still provide an option to solve the issues here.
- π¬π§United Kingdom joachim
Actually, I think the contrib testing modules problem might work already:
if (!isset($parsed_info['core_version_requirement'])) { if (str_starts_with($filename, 'core/') || str_starts_with($filename, $this->root . '/core/')) { // Core extensions do not need to specify core compatibility: they are // by definition compatible so a sensible default is used. Core // modules are allowed to provide these for testing purposes. $parsed_info['core_version_requirement'] = \Drupal::VERSION; } elseif (isset($parsed_info['package']) && $parsed_info['package'] === 'Testing') { // Modules in the testing package are exempt as well. This makes it // easier for contrib to use test modules. $parsed_info['core_version_requirement'] = \Drupal::VERSION; }
Doesn't that mean that if a contrib module a) is in the Testing package and b) does NOT specify core_version_requirement, then it's automatically taken to be compatible?
- πΊπΈUnited States cmlara
Doesn't that mean that if a contrib module a) is in the Testing package and b) does NOT specify core_version_requirement, then it's automatically taken to be compatible?
That is what its suppose to do per the last CR on the subject https://www.drupal.org/node/3070687 β .
Some contrib do have a core_version_requirement specified as the exemption wasn't available in the initial 8.7.7 release. In some cases I've seen it carry over when unneeded, and in other cases some contrib still has API that is compatible with 8.7.7 and as such declares compatability.
Expanding on my suggestion from #57, a setting could be done so that it can always install, even if in the testing package with core_version_requirement set.
BTW: since this is in the InfoParser, does this impact how modules display in the UI? I would think this might remove any warning from the modules page that the module is 'incompatible' making it harder for a developer to realize how far from mainline they have diverged.
Perhaps the bypass should be done in the Module Installer and Modules UI instead?
- π¬π§United Kingdom joachim
> Some contrib do have a core_version_requirement specified as the exemption wasn't available in the initial 8.7.7 release
Note that this is for contrib *Testing* modules. Not the main module in a project.
> does this impact how modules display in the UI
Testing modules normally don't show in the UI.
- πΊπΈUnited States cmlara
Testing modules normally don't show in the UI.
This code appears to impact non-testing root modules which would show up in the UI.
if (str_ends_with(\Drupal::VERSION, '-dev')) { // We skip extensions in the 'Testing' package, because there fixture // modules which are specifically for tests to check for an extension // being incompatible. if (!isset($parsed_info['package']) || $parsed_info['package'] !== 'Testing') { $parsed_info['core_version_requirement'] .= ' || ' . \Drupal::VERSION; } }
Note that this is for contrib *Testing* modules. Not the main module in a project.
Correct. The core_version_requirement may be set in a testing module because the parent module needed to test D8.7.7-8.8.1 (inclusive). Some modules never removed it even though they no longer test against that old a version while some modules may still test against D8.7.7.
Here is an example where I just didn't know I could omit core_version_requirement from a test module.
My knowledge having improved, next time I'm in that file I will remove it (since that branch only needs to test D9+) however it is reasonable to to believe other modules will have a similar setup.
- π¬π§United Kingdom longwave UK
I am starting to like the idea of a new setting as proposed in #57. This is both testable and predictable, and we have precedent with the setting that allows test modules to show in the UI.
However the downside is that users may start to enable it in order to accomplish major version upgrades, and then encounter (and report) bugs that are due to incompatibilities they have deliberately put in place.
Composer solves this problem by allowing per-package version aliases in composer.json. Would something similar work here? A setting containing an array of module names, that allows you to explicitly ignore/override
core_version_requirement
for those modules only? - π¬π§United Kingdom joachim
> Here is an example where I just didn't know I could omit core_version_requirement from a test module.
You couldn't before.
With this MR, you now *should* if you want your module to be testable on 11.x. Hmm, that means we would need to skip the check for testing modules on other versions too. Argh, this issue is like Laurel & Hardy pipework :(
> A setting containing an array of module names, that allows you to explicitly ignore/override core_version_requirement for those modules only?
That's going to be a pain because a developer would have to keep adding to it for further modules they want to use on 11.x.
> However the downside is that users may start to enable it in order to accomplish major version upgrades
People are going to shoot themselves in the foot no matter what anyway. We can put some VERY LOUD documentation on the setting to explain that it's meant for contrib development work.
> I am starting to like the idea of a new setting as proposed in #57.
It's certainly less hacky than the current MR! It does mean an extra check of settings and an extra setting value loaded into memory.
- πΊπΈUnited States cmlara
Adding on to my comment in #61
One more realization, would tampering at the InfoParser impact modules like upgrade_status that want to check on the modules and report? Is that another argument that this check should be done in the module installer and modules install form instead of in the InfoParser?
Bonus to @joachim note, likely less calls to the new setting as it only needs to requested when an install is occurring (though it will still be in memory from settings.php) if done outside the parser.
However the downside is that users may start to enable it in order to accomplish major version upgrades, and then encounter (and report) bugs that are due to incompatibilities they have deliberately or accidentally put in place.
A fair concern, I would suggest if we don't tamper with the info.yml the modules page will already show the module is incompatible, it could also be modified to add a header warning that incompatible modules have been allowed to install with the configuration override.
IIRC the status report already reports incompatible modules? If so if we don't tamper with the parser that would be another method for site owners to identify they are running outside of spec correct?
Beyond that I almost think that using this for site upgrades might be a useful feature. Using a config setting isn't much different than site owners patching the info.yml, at least with this method we would "know" that a module is being installed without patches.
All of the above are just suggestions, really for me as long as its not branch specific logic(use a setting) I'm mostly neutral on the proposal in general.
- Merge request !8615#3387322 - New setting to allow contrib extensions on 11.x β (Open) created by joachim
- Status changed to Needs review
5 months ago 3:44pm 1 July 2024 - π¬π§United Kingdom joachim
Created a new branch & MR with the new approach.
I've not added any of the changes to tests yet, as some if not all won't be necessary.
- Status changed to Needs work
5 months ago 7:19pm 1 July 2024 - πΊπΈUnited States cmlara
From comment #61:
Why are we changing this in the InfoParser instead of the MoudleInstaller? - Status changed to Needs review
5 months ago 7:13am 2 July 2024 - π¬π§United Kingdom joachim
Because InfoParserDynamic is where it's determined whether an extension is suitable for installation:
$parsed_info['core_incompatible'] = !Semver::satisfies(\Drupal::VERSION, $parsed_info['core_version_requirement']);
If we let 'core_incompatible' be set, then we have to act in a LOT more places. Not just ModuleInstaller, but every else that checks that key.
- πΊπΈUnited States cmlara
Good reason. I had it in my mind that it was done in the module handler.
For the second half of #61 could this instead be something like:
$is_compatible = Semver::satisfies(\Drupal::VERSION, $parsed_info['core_version_requirement']); $parsed_info['core_incompatible'] =!( $is_compatible || $ignore_core_version );
(where $ignore_core_version is set to TRUE earlier in the run instead of appending the current core version to the parsed data)
Goal being to avoid tamping with the data that is used elsewhere (like UI) to avoid hiding the fact the module is not actually current version compatible even though itβs installed.
- π¬π§United Kingdom joachim
> For the second half of #61 could this instead be something like:
Good idea! Much cleaner than fiddling with the version string.
I've made that change.
- Status changed to Needs work
5 months ago 9:32pm 4 July 2024 The Needs Review Queue Bot β tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide β to find step-by-step guides for working with issues.
- Status changed to Needs review
5 months ago 9:34pm 4 July 2024 - Status changed to Needs work
5 months ago 10:27pm 4 July 2024 The Needs Review Queue Bot β tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide β to find step-by-step guides for working with issues.
- π¬π§United Kingdom joachim
@quietone I'd specifically put 'enabled' because 'installed' is ambiguous - people were thinking this issue was about installing with Composer, when it's not.
- Status changed to Needs review
5 months ago 8:41am 5 July 2024 - π©π°Denmark ressa Copenhagen
Thanks for all the great work @joachim, I very much look forward to this feature! I could really have used it the last few months for Admin Toolbar, which is still not Drupal 11 ready ( π Add Drupal 11 support Needs review ).
I do agree with @quietone that "install" is the correct word to use here then, since "download = Composer". I recently posted this comment:
It's important to discern between downloading and installing modules, they are two very different things in Drupal.
- You download the module and its additional necessary libraries (defined in the module's composer.json file) with Composer.
- Then you install the module either via the GUI on the Extend page, or with Drush -- the module gets enabled, changes are made to the database (the configuration), necessary folders are created, etc.
See https://www.drupal.org/docs/extending-drupal/installing-modules β as well as these two issues for more discussions:
From Download β install β .
I have added a few words in the Issue Summary, to make the focus clear.
- π¬π§United Kingdom joachim
> You download the module and its additional necessary libraries (defined in the module's composer.json file) with Composer.
I very much support efforts to define and clarify terminology, but it's going to be hard in this case because Composer itself talks of 'Installing dependencies' and has the command `composer install`.
So I think we need to disambiguate the term 'install' in Drupal.
- π©π°Denmark ressa Copenhagen
Yes, it's a bit of a mess ... In the WordPress Plugins GUI, you press "Install", which in this case means both "download AND install".
If you check the Project Browser issue, "download" and "install" were chosen for these two steps. But let's continue the discussion by all means, and disambiguate.
Until a consensus has been reached, we probably need to be fairly precise when talking about different stages of module handling, to make sure everyone knows which part is in focus.
- πΊπΈUnited States cmlara
Visually (review of code action only) looks good to me, and appears to address my previously raised concerns.
Question on the feature side of this: You mention test work, does that include adding a flag to KernelTestBase and above to allow this to be easily enabled for phpunit runs? Possibly pair it with an environment variable for setup in phpunit.xml or GitlabCi?
- π¬π§United Kingdom joachim
Ah right, you mean so that a kernel test in a contrib module will accept to enable that module in the test site?
I hadn't actually thought of that! I'd like to get this in and leave that to a follow-up!
- π·π΄Romania claudiu.cristea Arad π·π΄
I've tested the MR on our project, with dozens of modules, from which >90% don't have an updated
core_version_requirement
. Works as expected by setting the kill-switch in settings.php. I wonder if this needs tests. Tentatively, marking as Needs tests - Status changed to Needs work
4 months ago 1:02pm 10 July 2024 - π·π΄Romania claudiu.cristea Arad π·π΄
Maybe we can extend
UpdateScriptTest::testExtensionCompatibilityChange()
orExtensionListTest::testCheckIncompatibility()
? - π¬π§United Kingdom joachim
joachim β changed the visibility of the branch 3387322-allow-contrib-extensions-on-11x to hidden.
- π¬π§United Kingdom joachim
joachim β changed the visibility of the branch 3387322-allow-contrib-extensions-on-11x-detect-dev-version to hidden.
- Status changed to Needs review
4 months ago 3:38pm 10 July 2024 - π¬π§United Kingdom joachim
Thanks for the hint! Got a unit test working.
- Status changed to Needs work
4 months ago 4:13pm 10 July 2024 The Needs Review Queue Bot β tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide β to find step-by-step guides for working with issues.
- Status changed to Needs review
4 months ago 4:39pm 10 July 2024 - Status changed to Needs work
4 months ago 7:12am 11 July 2024 - π·π΄Romania claudiu.cristea Arad π·π΄
Thank you for adding tests. Apart from a leftover (see the MR), I think we need:
- A change record
- Prepare a section in https://www.drupal.org/docs/upgrading-drupal/upgrading-from-drupal-8-or-... β , to explain how to use the lenient Composer plugin together with the new setting. Let's add this addition here and we'll update docs page after this is merged.
- Status changed to Needs review
4 months ago 10:35am 15 July 2024 - π¬π§United Kingdom joachim
Not sure about the test failures -- web/core/modules/system/tests/src/Functional/Menu/LocalTasksTest.php is failing for me locally on 11.x
- Status changed to Needs work
4 months ago 11:45am 19 July 2024 The Needs Review Queue Bot β tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide β to find step-by-step guides for working with issues.
- Status changed to Needs review
4 months ago 3:39pm 5 August 2024 - Status changed to Needs work
about 1 month ago 1:19pm 10 October 2024 The Needs Review Queue Bot β tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide β to find step-by-step guides for working with issues.