The core/scripts/switch-psr4.sh script was introduced in #2083547: PSR-4: Putting it all together as a tool to move PSR-0 module class files to PSR-4.
It is designed to handle both the $module_dir/lib/Drupal/$module_name/ and the $module_dir/tests/Drupal/$module_name/Tests/ directories.

Unfortunately, it seems the current version does not properly clean up empty "lib" directories.

Steps to reproduce:
1. Open drupal root in the commandline.
2. Run php core/scripts/switch-psr4.sh.
3. Run find | grep "lib$".

Operation 3. shows that the empty lib folders are leftover, instead of being removed.
ls -lA core/modules/system/lib shows these directories are really empty.

This is a blocker for #2247991: [May 27] Move all module code from …/lib/Drupal/… to …/src/… for PSR-4, therefore a "major" bug.

CommentFileSizeAuthor
#2 D8-2273689-2-switch-psr4-sh-simple.patch383 bytesdonquixote
#1 D8-2273689-1-switch-psr4-sh.patch4.5 KBdonquixote
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

donquixote’s picture

donquixote CreditAttribution: donquixote commented
Status: Active » Needs review
FileSize
D8-2273689-1-switch-psr4-sh.patch4.5 KB

I should mention that this is absolutely not covered by unit tests, and we don't intend to change that, since the script will be removed soon.

https://github.com/donquixote/drupal/compare/D8-2273689-1-switch-psr4-sh
https://github.com/donquixote/drupal/blob/D8-2273689-1-switch-psr4-sh/co...

donquixote’s picture

donquixote CreditAttribution: donquixote commented
FileSize
D8-2273689-2-switch-psr4-sh-simple.patch383 bytes

This goes a lot simpler, if it is only about the leftover "lib" directory.

donquixote’s picture

donquixote CreditAttribution: donquixote commented

The patch in #2 will NOT remove any leftover "lib" directories from prior operations, but this is actually ok.

Btw, I imagine some of the functions could live on as static utility methods in Drupal? Where would be the place?

ngocketit’s picture

ngocketit CreditAttribution: ngocketit commented
Issue tags: +drupalcampfi
ngocketit’s picture

ngocketit CreditAttribution: ngocketit commented
Assigned: Unassigned » ngocketit
ngocketit’s picture

ngocketit CreditAttribution: ngocketit commented
Assigned: ngocketit » Unassigned

The patch in #2 is indeed a lot simpler and does what it's supposed to do: removing leftover "lib" folders. I think we only need to run the script once so the issue with leftover "lib" from prior operations shouldn't be a problem.

Btw, I imagine some of the functions could live on as static utility methods in Drupal? Where would be the place?

Do you think we can reuse some functions in the script elsewhere?

ngocketit’s picture

ngocketit CreditAttribution: ngocketit commented
Assigned: Unassigned » ngocketit
ngocketit’s picture

ngocketit CreditAttribution: ngocketit commented
Assigned: ngocketit » Unassigned
xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Priority: Major » Critical
Status: Needs review » Reviewed & tested by the community
Issue tags: -drupalcampfi +beta blocker

So this is also the LITTLEST BETA BLOCKER EVER because this bug keeps the script from working properly for #2247991: [May 27] Move all module code from …/lib/Drupal/… to …/src/… for PSR-4 and the subsequent patch rerolls.

Btw, I imagine some of the functions could live on as static utility methods in Drupal? Where would be the place?

I don't think that's necessary.

Sorry @ngocketit for stealing this back but it's important to just get in. :) Thanks for the review!

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Issue tags: +drupalcampfi

Oops.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Committed 9ad1cda and pushed to 8.x. Thanks!

  • Commit 9ad1cda on 8.x by alexpott: 
    Issue #2273689 by donquixote: Fixed switch-psr4.sh does not reliably...

Status: Fixed » Closed (fixed)

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

iMiksu’s picture

iMiksu
Location Finland
CreditAttribution: iMiksu as a volunteer commented
Issue tags: -drupalcampfi

Cleaning up drupalcampfi tags.