Skip to content

also load app autoloader when located in /vendor#23125

Closed
icewind1991 wants to merge 1 commit intomasterfrom
autoload-app-vendor
Closed

also load app autoloader when located in /vendor#23125
icewind1991 wants to merge 1 commit intomasterfrom
autoload-app-vendor

Conversation

@icewind1991
Copy link
Member

@icewind1991 icewind1991 commented Oct 1, 2020 •
edited
Loading

with the phasing out of app.php apps can no longer load the autoloader from there themselves

See #21812

@icewind1991
also load app autoloader when located in /vendor
e062c42 
Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991 icewind1991 added the 3. to review Waiting for reviews label Oct 1, 2020
@icewind1991 icewind1991 added this to the Nextcloud 21 milestone Oct 1, 2020
@icewind1991
Copy link
Member Author

icewind1991 commented Oct 1, 2020

/backport to stable20

@faily-bot
Copy link

faily-bot bot commented Oct 1, 2020

🤖 beep boop beep 🤖

Here are the logs for the failed build:

Status of 33536: failure

jsunit

Show full log 
PhantomJS 2.1.1 (Linux 0.0.0) WARN: 'No OC found'
PhantomJS 2.1.1 (Linux 0.0.0) LOG: 'JQMIGRATE: Migrate is installed, version 1.4.1'
PhantomJS 2.1.1 (Linux 0.0.0) WARN: 'jQuery is deprecated: The global jQuery is deprecated. It will be updated to v3.x in Nextcloud 21. In later versions of Nextcloud it might be removed completely. Please ship your own.'
PhantomJS 2.1.1 (Linux 0.0.0) WARN: '$ is deprecated: The global jQuery is deprecated. It will be updated to v3.x in Nextcloud 21. In later versions of Nextcloud it might be removed completely. Please ship your own.'
PhantomJS 2.1.1 (Linux 0.0.0) WARN: 'Handlebars is deprecated: please ship your own, this will be removed in Nextcloud 20'
PhantomJS 2.1.1 (Linux 0.0.0) DEBUG: 'OCA.Files.Settings initialized'
PhantomJS 2.1.1 (Linux 0.0.0) DEBUG: 'OCA.Files.Sidebar initialized'
PhantomJS 2.1.1 (Linux 0.0.0) DEBUG: 'OCA.Sharing.ShareSearch initialized'
PhantomJS 2.1.1 (Linux 0.0.0) DEBUG: 'OCA.Sharing.ExternalLinkActions initialized'
PhantomJS 2.1.1 (Linux 0.0.0): Executed 847 of 850 (skipped 3) SUCCESS (11.135 secs / 10.562 secs)

@ChristophWurst
Copy link
Member

ChristophWurst commented Oct 2, 2020

with the phasing out of app.php apps can no longer load the autoloader from there themselves

But you can require_once from the header of Application.php, no?

@MorrisJobke
Copy link
Member

MorrisJobke commented Oct 6, 2020

with the phasing out of app.php apps can no longer load the autoloader from there themselves

But you can require_once from the header of Application.php, no?

Or just put it in composer/autoloader.php?

juliusknorr
juliusknorr approved these changes Oct 6, 2020
View reviewed changes
Copy link
Member

@juliusknorr juliusknorr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 from my side since vendor is actually the default target directory for installing composer packages without additional configuration of the vendor-dir

@ChristophWurst ChristophWurst added the pending documentation This pull request needs an associated documentation update label Nov 10, 2020
ChristophWurst
ChristophWurst approved these changes Nov 10, 2020
View reviewed changes
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well fine I guess

@ChristophWurst
Copy link
Member

ChristophWurst commented Nov 10, 2020

argh I'm so torn on this one. we have to check the file existence for every app. the os/fs will cache the result but still we do this. and if the require_once is in application.php (and we properly document this) it's explicit and less magical.

@MorrisJobke
Copy link
Member

MorrisJobke commented Nov 10, 2020

and if the require_once is in application.php (and we properly document this) it's explicit and less magical.

Sounds sane 👍

ChristophWurst
ChristophWurst requested changes Nov 10, 2020
View reviewed changes
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then I'll change my mind one more time. I'll do the docs update.

This was referenced Nov 24, 2020
Merged
Merged
This was referenced Dec 14, 2020
Merged
Merged
This was referenced Dec 28, 2020
Merged
Merged
@rullzer rullzer modified the milestones: Nextcloud 21, Nextcloud 22 Jan 6, 2021
@icewind1991 icewind1991 deleted the autoload-app-vendor branch March 26, 2021 14:27
@ChristophWurst ChristophWurst removed the pending documentation This pull request needs an associated documentation update label Jul 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ChristophWurst ChristophWurst ChristophWurst requested changes

@juliusknorr juliusknorr juliusknorr approved these changes

@rullzer rullzer Awaiting requested review from rullzer

@MorrisJobke MorrisJobke Awaiting requested review from MorrisJobke

Assignees

No one assigned

Labels

3. to review Waiting for reviews

Projects

None yet

Milestone

Nextcloud 22

Development

Successfully merging this pull request may close these issues.

5 participants

@icewind1991 @ChristophWurst @MorrisJobke @juliusknorr @rullzer