Posted by Berdir on April 21, 2012 at 11:57am
31 followers
| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | base system |
| Category: | task |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
| Issue tags: | API change, API clean-up, PSR-0 |
Issue Summary
I couldn't resist. :)
This should work once we're done with #1513210: Meta: Start converting module provided classes to PSR-0.
Right now this fails with "Fatal error: Class 'DrupalDefaultEntityController' not found in ../core/modules/node/node.module on line 4143"
I am hopeful that the front page works once the entity stuff is all (or at least user and node and obviously entity.module) converted to PSR-0.
Note that we should also remove modules/system/tests/registry.test with this.
Change records for this issue
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| remove_registry.patch | 30.55 KB | Idle | FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed. | View details |
Comments
#1
The last submitted patch, remove_registry.patch, failed testing.
#2
Re-rolled. This patch currently includes the changes from #1495024: Convert the entity system to PSR-0.
I am now able to run the Tracker test (the only one that is currently converted to PSR-0) with this patch. Let's see if the test bot is too.
#3
#4
1280 deletions, 663 insertions... I love this patch. Berdir gets 2:1 high fives. This is proof that adopting PHP standards is a good thing for Drupal.
#5
Actually, that's just because the entity psr-0 patch was part of the conversion. The real delete ratio is more like this:
12 files changed, 7 insertions(+), 712 deletions(-)Re-rolled.
#6
The last submitted patch, remove-registry-1541674-5.patch, failed testing.
#7
I mentioned this to merlinofchaos and he pointed out that it will mean a lot of conversions to do in views (or any other contrib module with a lot of classes) to port it to 8.x once the registry is removed.
That's got to happen at some point, but I think it makes sense to schedule the actual commit of this patch so we can warn anyone else who might attempt a port. Not sure exactly when but was thinking either August 1st or September 1st (I doubt core classes will be fully converted for another 2-3 weeks and I'm going to be mostly afk the second half of June, so really that'd either be an extra 4 weeks or 8 weeks beyond when this could otherwise get committed.
#8
Sonds fine to me, this isn't blocking something else, as long as we can port our own classes to PSR-0 asap, then we can throw this out whenever we feel like it. E.g. after the plugin system stuff lands, which Views will need to port to anyway.
#9
Here's a re-roll, let's see how far we are.
#10
+++ b/core/modules/simpletest/simpletest.module@@ -288,13 +288,9 @@ function simpletest_log_read($test_id, $prefix, $test_class, $during_test = FALS
+ * The list of test classes is loaded by searching the designated directory ¶
+ * for each module for files matching the PSR-0 standard. Once loaded the ¶
+ * test list is cached and stored in a static variable. ¶
trailing white-spaces
#11
Lots of the remaining tests are in #1593058: Remove system.info's files[] entry. Adding the tag.
#12
Closing #1598610: Convert symfony.test to PSR-0 and remove all the files[] instances in system.info. Remember to also remove the PSR-0 class loading tests in symfony.test, that wouldn't even be executed, if PSR-0 wasn't working.
#13
#9: remove-registry-1541674-9.patch queued for re-testing.
How're we looking? I know there are still patches to be committed, just wanted to see what the test-bot had to say.
#14
The last submitted patch, remove-registry-1541674-9.patch, failed testing.
#15
Rerolled #9 out of curiosity.
#16
The last submitted patch, remove-registry-1541674-15.patch, failed testing.
#17
Ah, the "list can not be used in namespace" problem. I guess this depends only on #1592632: Merge List field types into Options module now.
#18
Given that we really want to remove the registry asap, I'm changing this to major and setting to postponed on #1592632: Merge List field types into Options module and until the views PSR-0 efforts are done.
#19
For reference, http://drupal.org/project/issues/search/views?issue_tags=PSR-0 .
#20
See also: #1667822: Remove caching of test classes
#21
The patch didn't apply anymore so I re-rolled #15 which is a re-roll of #9.
#22
The last submitted patch, remove-registry-1541674-21.patch, failed testing.
#23
This was postponed both on the List->Options issue, and on Views PSR-0. The first is done, the second is not (it was until recently blocked on plugins).
Re-postponing for now.
#24
Views should not block this removal. Last I checked and talked to @dawehner, it was using PSR-0 tests already.
The main problem for Views and all other contributed modules was the PIFR issue #1674290: PSR-0 tests in contrib modules are not found, which has been fixed already and just needs to be deployed.
#25
The reason I rerolled it was to see what failed, so that it can be worked on, even in the postponed state (or not). Should have made that clear.
#26
I don't think we should postpone it on Views, but I do think we should schedule this commit to give people fair warning if there are contrib projects chasing HEAD since it's not a hard patch to get in and doesn't hold anything up. What about 31st August?
#27
Fine with me, could also do it during DrupalCon a few days earlier, then we'll have a reason to party ;)
Will try to get it back to green asap. I think should actually be needs work, the last patch failed.
#28
I went through some of the most used projects on Drupal.org, and it looks like only Admin Menu is affected by this. I've put up #1691418: Update tests to PSR-0 to track that. Love this to go in sooner rather than later though, as I'm sure we have some follow ups to address.
[EDIT] sun reported:
So, it looks like the only blockers are:
#29
Why are we only talking about test classes?
Views is 99% done converting its test classes, but hasn't begun on the 250+ non-test classes.
Also,
$ grep -nr "^files\[\]" corecore/modules/field/tests/modules/field_test/field_test.info:5:files[] = field_test.entity.inc
core/modules/system/system.info:10:files[] = tests/registry.test
core/modules/system/tests/modules/file_test/file_test.info:6:files[] = file_test.module
#30
Can we move the registry back over to http://drupal.org/project/registry for Drupal 8? That way, if a contrib module needs it, then it can simply depend on it until it fully moves to PSR-0. If Drupal Core doesn't need the registry, then why should it be in Drupal Core?
#31
If someone wants to do that, then he can :)
The registry is IMHO not blocking anything, we don't need to get it out asap. We just want to get it out before API freeze/feature freeze/whatever point of no return it will be. damiankloip confirmed that he expects to have converted Views to PSR-0 by 31. August.
I will make sure that the patch is ready by then. Leaving it at needs work, will set it to postponed once the tests are green again.
Ok? :)
#32
Sounds good to me.
#33
Sounds good!
#34
Ok, this should pass I think.
Had to remove the registering and unregistering of the autload functions in the upgrade tests. Also added an update function to remove the registry tables.
#35
#36
Green. We can re-test it shortly before the defined date....
#37
#1716048: Do not boot bundle classes on every request, can't release with this in.
#38
If the Annotations patch goes in, Views is done. Then we don't have to wait until August 31st.
EDIT:
By "Views is done", I mean "Views no longer needs the registry". :)
#39
#1683644: Use Annotations for plugin discovery is in.
#40
#34: remove-registry-1541674-34.patch queued for re-testing.
#41
The last submitted patch, remove-registry-1541674-34.patch, failed testing.
#42
Re-rolled against HEAD.
Aside from conflicts and shifted hunks, no change except of renumbered system update. interdiff contains everything (but the shifted + conflicting hunks are misleading).
#43
That interdiff really isn't helpful, so here's a direct patch-to-patch diff.
#44
This patch is great. The doc comments make complete sense for what is happening in SimpleTest, and the caches being cleared is needed. RTBC.
#45
I would like to postpone this on #1724452: Test Annotations sandbox. Views is almost done converting his classes to plugins but we need the views testbot at this stage. When that conversion is passing tests we can merge in and we don't have to rely anymore on the registry.
Ok? Thanks :)
#46
It's also worth noting that CTools' Export UI is not yet converted to the new plugin system and PSR-0 and it relies on the registry. While it shouldn't be a difficult conversion it'll still take a little time and might be tough to get done before Munich at this point.
#47
I said 31st August back in July, and I'm still happy to leave it until then?
#48
+++ b/core/modules/simpletest/simpletest.module@@ -288,13 +288,9 @@ function simpletest_log_read($test_id, $prefix, $test_class, $during_test = FALS
+ * The list of test classes is loaded by searching the designated directory
+ * for each module for files matching the PSR-0 standard. Once loaded the
+ * test list is cached and stored in a static variable.
Wrong comment wrapping. Rerolled the patch, no commit credit, please :)
#49
Until August 31st.
#50
Can we announce this at g.d.o/core? This is going to impact a large number of folks IMO.
#51
Done: http://groups.drupal.org/node/248853
#52
#48: rr-1541674-48.patch queued for re-testing.
#53
The last submitted patch, rr-1541674-48.patch, failed testing.
#54
Re-rolled, will set back to postponed once it comes back green.
#55
The last submitted patch, rr-1541674-54.patch, failed testing.
#56
Forgot a }.
#57
Green, back to sleep with you.
#58
1 day until scheduled commit. So, simple re-roll to catch up with HEAD and to have the test bot look at it one last time.
#59
Re-roll looks good, bot is green. Awesome! Let's do this.
#60
Confirmed that CTools and Views are no longer using the registry.
#61
Rerolled for #1468328: Move file entity info, managed file, and file usage functionality into File module
#62
The last submitted patch, registry-1541674-61.patch, failed testing.
#63
#61: registry-1541674-61.patch queued for re-testing.
#64
The fail in #62 was because I submitted it too early :)
Re-RTBCing.
#65
I would commit this, but I'm pretty sure catch really wants to, so leaving this for him for a few hours. :)
#66
Yes, yes I did quite want to. Committed/pushed to 8.x, thanks all!
I think http://drupal.org/node/1320394 covers this for the change notice so moving straight to fixed.
#67
Added a back-reference to http://drupal.org/node/1320394 and mentioned in the last sentence there that the registry has now actually been removed.
#68
Big win! :)
#69
Woohoo! Great work everyone.
#70
Automatically closed -- issue fixed for 2 weeks with no activity.
#71
Clean up: #1810686: Remove the remaining files[] entries
#72
Automatically closed -- issue fixed for 2 weeks with no activity.