Problem/Motivation

The doctrine/annotations package has been marked a abandoned: https://github.com/doctrine/annotations/commit/a64d7f063e1602bbbc8e70a07...

Since #3252386: Use PHP attributes instead of doctrine annotations we support native attributes but drupal/core still depends on doctrine/annotations.

Steps to reproduce

Run composer install, you get this warning:

Package doctrine/annotations is abandoned, you should avoid using it. No replacement was suggested.

Proposed resolution

Convert every remaining annotation to attributes and remove doctrine/annotations from dependencies.

Copy the code that we use into a core namespace and remove doctrine/annotations from dependencies.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

The doctrine/annotations package is marked abandoned upstream. Parts of the code have been forked into Drupal core. This change only affects custom code that was handling PHP annotations; for more information, see the change record.

CommentFileSizeAuthor
#16 3550917-16.patch58.08 KBjohnatas

Issue fork drupal-3550917

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

prudloff created an issue. See original summary.

prudloff’s picture

catch’s picture

We deprecated annotations already, but for removal in Drupal 13. https://www.drupal.org/node/3522776

The only remaining annotations usages in Drupal 11 are in some migrate plugins (which are going to be removed in Drupal 12) and the bc layer for annotations.

For Drupal 11, I think our only option is to fork Doctrine Annotations and un-abandon it to suppress the warning, we've had to fork entire dependencies before (for the Zend/Laminas change iirc).

For Drupal 12, we have #3400121: Allow opt-out of annotation parsing to make annotation discovery opt-out, we could also make it opt-in or bring forward the Drupal 13 removal to Drupal 12, although that would need an announcement and it would need to be announced quickly.

As @berdir pointed out in that issue the security surface is minimal, the main issue is support for newer PHP versions, which could get worse the further into Drupal 12's release cycle we are.

catch’s picture

Priority: Normal » Critical

Moving this to critical, while we will end up with the same code running or not running, it would be good to resolve this asap especially since this is likely to lead to lots of duplicate reports.

longwave’s picture

We forked parts of Doctrine already by copying them into core namespaces, so I think the simplest solution is to do that for the remaining classes that we use.

ghost of drupal past’s picture

As the author of some of that code: you could also delete some code after forking, after all Drupal only ever supported class annotations in proper PSR namespaces.

longwave’s picture

Status: Active » Needs review

Copied all the classes that we use into core; DocParserTest still passes locally, let's see if anything else is broken. Unsure if we should copy any other tests from Doctrine.

longwave’s picture

Once again I am surprised that this is green so easily.

quietone’s picture

Something is amiss, the link for the CR in #10 is a 404 for me. I searched the change records as well and didn't find one for this.

longwave’s picture

Status: Needs review » Needs work

Let's try to add some backward compatibility via moved_classes and/or class_alias().

godotislate’s picture

Draft: https://www.drupal.org/node/3551049

This is a 403 for me. And I don't see a link the CR in the sidebar here.

berdir’s picture

YesCT might have administer node permission or something on d.o and mixed up the draft and published checkboxes, has happened before as. Someone else with enough permissions should able to publish it while keep it a draft?

longwave’s picture

Published the change record node (but not the change record itself)

johnatas’s picture

StatusFileSize
new58.08 KB

Hello,

Thank you very much for the work.

For those who might be interested, I’m attaching a patch that includes the current changes from the MR !13433 (core directory only).

berdir’s picture

You can't patch composer dependencies, there is no point in using (or uploading) patch files here. Nothing is broken, nothing is insecure, this is just critical exactly because a lot of people are going to be asking questions about this and we want it resolved quickly. Ignore the warning and stay tuned.

loopy1492’s picture

Right now we are auditing composer to fail on abandoned packages. Is the Drupal security team's assessment that it is safe to skip this error on our validation/audit steps in CI?

longwave’s picture

Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs change record, -Needs release note

Added a BC layer for AnnotationException, and updated the change record.

I am not sure we should provide BC for anything else here. If you were depending on doctrine/annotations, and you required it in composer.json: your code will still work. If you were depending on doctrine/annotations as a transitive dependency of core, I am not sure that is core's responsibility to fix.

godotislate’s picture

Status: Needs review » Reviewed & tested by the community

I compared the Drupal\Component\Annotation\Doctrine\* classes and to Doctrine\Tests\Common\Annotations\* 2.0.2 classes and confirmed that the code is essentially identical.

BC for exception looks good. Also, if this is backported to D10, I think the CR alone is enough without any BC.

LGTM.

xjm’s picture

xjm’s picture

xjm’s picture

Issue tags: +10.6.0 release notes
xjm’s picture

Discussed with @catch and we would like to backport this to 10.6 as well.

longwave’s picture

Issue summary: View changes
yesct’s picture

FYI, I saw Audit: add option to ignore individually abandoned packages #12572 https://github.com/composer/composer/pull/12572 was just merged.

catch’s picture

Status: Reviewed & tested by the community » Needs work

This looks good but it needs a rebase.

longwave’s picture

Status: Needs work » Reviewed & tested by the community

Rebased, back to RTBC.

  • catch committed 20de6353 on 11.3.x
    Issue #3550917 by prudloff, longwave, yesct, godotislate, berdir:...

  • catch committed 19d25e6b on 11.x
    Issue #3550917 by prudloff, longwave, yesct, godotislate, berdir:...
catch’s picture

Version: 11.x-dev » 10.6.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed/pushed to 11.x and cherry-picked to 11.3.x, thanks!

Moving to 10.6.x for backport.

godotislate’s picture

doctrine/annotations is still in the composer.lock, which is causing test failures on HEAD #3554820: HEAD testing is broken.

  • catch committed c2aa62a6 on 11.x
    Issue #3550917 by prudloff, longwave, yesct, godotislate, berdir:...
catch’s picture

Committed the follow-up, PHPStan is happy in the latest branch job now.

godotislate’s picture

10.6.x is locked to doctrine/annotations 1.14.4, so should the Drupal forked classes in 10.6.x be copies of that version instead of 2.0.2?

mondrake’s picture

I think this should be cherrypicked to 11.3.x too

longwave’s picture

@godotislate not too sure it matters given we are changing the namespaces anyway, can't remember what the break was between annotations 1 and 2. Also not sure what to do about the exception BC layer though as that isn't supported in the same way in D10.

godotislate’s picture

Attempted a backport to 10.6 on MR 13610 using doctrine/annotations 1.14.4, but ran into some phpstan issues. Will have to loop back later to investigate.

catch’s picture

I thought I'd cherry-picked to 11.3.x but apparently not, done now.

godotislate’s picture

Fixed the PHPStan issues, but tests are failing on the 10.6 MR.

godotislate’s picture

Status: Patch (to be ported) » Needs review

https://git.drupalcode.org/project/drupal/-/merge_requests/13610#note_61... for 10.6 is ready.

@catch re: #42 I think the 11.3.x commit might not have been pushed. Maybe it happened during the Gitlab maintenance period?

  • catch committed 920f7e1a on 11.3.x
    Issue #3550917 by prudloff, longwave, yesct, godotislate, berdir:...

quietone’s picture

Status: Needs review » Patch (to be ported)

@godotislate, thanks for making the 10.6 MR

godotislate’s picture

Is the 10.6 MR good to go to RTBC?

longwave’s picture

Status: Patch (to be ported) » Reviewed & tested by the community

10.6 MR looks good to me. We could class_alias() the exception if we think it's worth it, but unsure if that comes with any risks if doctrine/annotations is still installed for some reason.

eduardo morales alberti’s picture

Hi, why is this merge into branch 11.x, but is not on the latest releases? like 11.2.8 https://git.drupalcode.org/project/drupal/-/blob/11.2.8/composer.lock?re...

Composer audit flags the package doctrine as deprecated, which is a security issue, and it has the value critical.
Is there any planned release with this change?

longwave’s picture

A deprecated package is not a security issue; it is a warning that the package will become unsupported in the future. We have deliberately not made this change in 11.2 because there is a chance it will break existing sites and we try to avoid that as much as possible in patch releases. It will be released with 11.3.0 in December.

  • catch committed 8b5da561 on 10.6.x
    fix: #3550917 doctrine/annotations is abandoned
    
    By: @prudloff
    By: @...

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 10.6.x, thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

eduardo morales alberti’s picture

Thank you for the explanation @longwave

longwave’s picture

Published the change record.

longwave’s picture

yesct’s picture

In case folks are waiting for the releases,
and to follow-up on my previous comment https://www.drupal.org/project/drupal/issues/3550917#comment-16323381

If folks' CI was blocking them, I wanted to share that people can add to composer.json

        "audit": {
            "ignore-abandoned": ["doctrine/annotations"]
        }

But I also want to note that, I recommend also adding a followup issue to some internal tracker (not d.o) to also remove that workaround once the d.o core releases come out. Which by looking at https://www.drupal.org/about/core/policies/core-release-cycles/schedule#... I'm guessing will be "Week of December 8, 2025 (UTC)"

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.