Problem/Motivation

Drupal core uses classes/interfaces/traits from psr/log package. However, that package is not listed as a requirement in core composer.json file. It gets installed just because some other Drupal dependencies require it.

$ composer why psr/log
composer/composer                1.9.1       requires  psr/log (^1.0)  
composer/xdebug-handler          1.4.0       requires  psr/log (^1.0)  
consolidation/annotated-command  4.0.0       requires  psr/log (^1)    
consolidation/log                1.1.1       requires  psr/log (^1.0)  
drush/drush                      dev-master  requires  psr/log (~1.0)  
symfony-cmf/routing              2.1.0       requires  psr/log (^1.0)  
symfony/debug                    v4.4.0-RC1  requires  psr/log (~1.0)  
symfony/error-handler            v4.4.0-RC1  requires  psr/log (~1.0)  
symfony/http-kernel              v4.4.0-RC1  requires  psr/log (~1.0)  
symfony/lock                     v4.4.0-RC1  requires  psr/log (~1.0)

Proposed resolution

Add psr/log: ^1.0 to core composer.json file.

CommentFileSizeAuthor
#21 3095948-add-psr-log-21-9.0.x-dev.patch1.2 KBamjad1233
#17 3095948-add-psr-log-17-9.0.x-dev.patch1.2 KBamjad1233
#14 3095948-add-psr-log-14.patch1.2 KBamjad1233
#12 3095948-add-psr-log-12.patch1.55 KBamjad1233
#9 3095948-add-psr-log-9.patch3.2 KBamjad1233
#6 3095948-add-psr-log-6.patch3.29 KBamjad1233
#2 3095948-add-psr-log-1.patch446 bytesamjad1233
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Chi created an issue. See original summary.

amjad1233’s picture

amjad1233
MJED
Location Brisbane
CreditAttribution: amjad1233 as a volunteer and commented
Assigned: Unassigned » amjad1233
Status: Active » Needs review
FileSize
3095948-add-psr-log-1.patch446 bytes
Chi’s picture

Chi CreditAttribution: Chi commented
Status: Needs review » Needs work

I suppose composer.lock also needs to be updated.

amjad1233’s picture

amjad1233
MJED
Location Brisbane
CreditAttribution: amjad1233 as a volunteer and commented

There was no composer.lock file in the repo. So I didn't put.

Chi’s picture

Chi CreditAttribution: Chi commented

It's in the root directory.
https://git.drupalcode.org/project/drupal/blob/8.9.x/composer.lock

amjad1233’s picture

amjad1233
MJED
Location Brisbane
CreditAttribution: amjad1233 as a volunteer and commented
FileSize
3095948-add-psr-log-6.patch3.29 KB

Oh. Sorry, this time I ran composer require psr/log:^1.0 where composer lock was, is this right?

I am not sure if I will rin composer require psr/log:^1.0 in core dir as described in the resolution section, how would it update the root composer.lock?

amjad1233’s picture

amjad1233
MJED
Location Brisbane
CreditAttribution: amjad1233 as a volunteer and commented
Status: Needs work » Needs review
kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
CreditAttribution: kim.pepper at PreviousNext commented 
+++ b/composer.json
@@ -13,7 +13,8 @@
+        "wikimedia/composer-merge-plugin": "^1.4",
+        "psr/log": "^1.0"

nit: can we add this in alphabetical order?

amjad1233’s picture

amjad1233
MJED
Location Brisbane
CreditAttribution: amjad1233 as a volunteer and commented
FileSize
3095948-add-psr-log-9.patch3.2 KB
amjad1233’s picture

amjad1233
MJED
Location Brisbane
CreditAttribution: amjad1233 as a volunteer and commented

@kim.pepper
Is it okay to raise an issue for adding sort-packages:true config? (https://git.drupalcode.org/project/drupal/blob/9.0.x/composer.json#L43) or would be overkill?

Chi’s picture

Chi CreditAttribution: Chi commented
Status: Needs review » Needs work

I am not sure if I will run composer require psr/log:^1.0 in core dir as described in the resolution section, how would it update the root composer.lock?

It's a bit more complicated. psr/log belongs to drupal/core package so it needs to be added to core/composer.json file.
I think you can run composer update drupal/core from the root directory to update the composer.lock file.

Is it okay to raise an issue for adding sort-packages:true config?

Yes.

amjad1233’s picture

amjad1233
MJED
Location Brisbane
CreditAttribution: amjad1233 as a volunteer and commented
FileSize
3095948-add-psr-log-12.patch1.55 KB

Here comes another one.

Thanks for your help.

Chi’s picture

Chi CreditAttribution: Chi commented 
+            "bin": [
+                "bin/composer"
+            ],

This seems irrelevant to the ticket.

amjad1233’s picture

amjad1233
MJED
Location Brisbane
CreditAttribution: amjad1233 as a volunteer and commented
FileSize
3095948-add-psr-log-14.patch1.2 KB

Sorry. Wonder how I missed this. Guess my habit of ignoring the composer.lock changes.

amjad1233’s picture

amjad1233
MJED
Location Brisbane
CreditAttribution: amjad1233 as a volunteer and commented
Status: Needs work » Needs review
Chi’s picture

Chi CreditAttribution: Chi commented
Status: Needs review » Reviewed & tested by the community

I think it's good now. Thank you.

amjad1233’s picture

amjad1233
MJED
Location Brisbane
CreditAttribution: amjad1233 as a volunteer and commented
FileSize
3095948-add-psr-log-17-9.0.x-dev.patch1.2 KB

Testing against Drupal 9. As this will be needed to be ported to 9.0.x/

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott as a volunteer and at Acro Commerce, Thunder commented
Status: Reviewed & tested by the community » Needs work

This needs a reroll at least on 9.0.x which is where it has to go first.

Chi’s picture

Chi CreditAttribution: Chi commented
Version: 8.9.x-dev » 9.0.x-dev
Status: Needs work » Reviewed & tested by the community
Issue tags: +Needs reroll
Chi’s picture

Chi CreditAttribution: Chi commented
Status: Reviewed & tested by the community » Needs work
amjad1233’s picture

amjad1233
MJED
Location Brisbane
CreditAttribution: amjad1233 as a volunteer and commented
Status: Needs work » Needs review
FileSize
3095948-add-psr-log-21-9.0.x-dev.patch1.2 KB

Here is an updated patch.

Chi’s picture

Chi CreditAttribution: Chi commented
Status: Needs review » Reviewed & tested by the community

#21 Looks good. Thank you.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott as a volunteer and at Acro Commerce, Thunder commented
Category: Bug report » Task
Status: Reviewed & tested by the community » Fixed
Issue tags: -Needs reroll

Committed 2d2cbce and pushed to 9.0.x. Thanks!
Committed c00078f and pushed to 8.9.x. Thanks!

Not bothered with 8.8.x because it's in RC and about to reach 8.8.0 and this is more a task than anything else. Not really a bug.

  • alexpott committed 2d2cbce on 9.0.x 
    Issue #3095948 by amjad1233, Chi: Require explicitly psr/log

  • alexpott committed c00078f on 8.9.x 
    Issue #3095948 by amjad1233, Chi: Require explicitly psr/log

Status: Fixed » Closed (fixed)

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