Skip to content

only setup part of the filesystem for appdata requests #30942

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Feb 10, 2022
Merged

only setup part of the filesystem for appdata requests #30942

merged 5 commits into from
Feb 10, 2022

Conversation

@icewind1991
Copy link
Member

Signed-off-by: Robin Appelman robin@icewind.nl

@icewind1991 icewind1991 added the 2. developing Work in progress label Jan 31, 2022
@icewind1991 icewind1991 added this to the Nextcloud 24 milestone Jan 31, 2022
@icewind1991 icewind1991 force-pushed the appdata-lite-filesystem branch 2 times, most recently from 3ba5f91 to 28662e3 Compare January 31, 2022 17:09
blizzz
blizzz reviewed Jan 31, 2022
View reviewed changes
@icewind1991 icewind1991 force-pushed the appdata-lite-filesystem branch 2 times, most recently from 3be90b1 to 1a8db39 Compare January 31, 2022 17:50
nickvergessen
nickvergessen reviewed Jan 31, 2022
View reviewed changes
nickvergessen
nickvergessen reviewed Feb 1, 2022
View reviewed changes
@icewind1991 icewind1991 force-pushed the appdata-lite-filesystem branch 6 times, most recently from 0cefccd to 5808492 Compare February 1, 2022 18:05
@juliusknorr juliusknorr mentioned this pull request Feb 2, 2022
Closed
@PVince81
Copy link
Member

PVince81 commented Feb 2, 2022 •
edited
Loading

cool, tests pass

so now to figure out what to retest.

some smoke testing of FS related code:

  • general
    • files_accesscontrol
    • cron jobs
    • group folders
    • sharing / receiving shares
    • external storage
    • text
    • richdocuments
    • occ files:scan
    • occ files:scan-app-data 🚫
  • app data
    • thumbnails
    • photos app
    • theming (for SCSS recompilation)

@PVince81
Copy link
Member

PVince81 commented Feb 2, 2022 •
edited by icewind1991
Loading

@icewind1991

  • BUG: when scanning app data folder:
% occ files:scan-app-data -vvvv
Scanning AppData for files

An unhandled exception has been thrown:
TypeError: Argument 1 passed to OC_Util::setupRootFS() must be of the type string, null given, called in /srv/www/htdocs/nextcloud-release/lib/private/legacy/OC_Util.php on line 319 and defined in /srv/www/htdocs/nextcloud-release/lib/private/legacy/OC_Util.php:189
Stack trace:
#0 /srv/www/htdocs/nextcloud-release/lib/private/legacy/OC_Util.php(319): OC_Util::setupRootFS()
#1 /srv/www/htdocs/nextcloud-release/lib/private/Files/Utils/Scanner.php(115): OC_Util::setupFS()
#2 /srv/www/htdocs/nextcloud-release/lib/private/Files/Utils/Scanner.php(201): OC\Files\Utils\Scanner->getMounts()
#3 /srv/www/htdocs/nextcloud-release/apps/files/lib/Command/ScanAppData.php(136): OC\Files\Utils\Scanner->scan()
#4 /srv/www/htdocs/nextcloud-release/apps/files/lib/Command/ScanAppData.php(171): OCA\Files\Command\ScanAppData->scanFiles()
#5 /srv/www/htdocs/nextcloud-release/3rdparty/symfony/console/Command/Command.php(255): OCA\Files\Command\ScanAppData->execute()
#6 /srv/www/htdocs/nextcloud-release/core/Command/Base.php(168): Symfony\Component\Console\Command\Command->run()
#7 /srv/www/htdocs/nextcloud-release/3rdparty/symfony/console/Application.php(1009): OC\Core\Command\Base->run()
#8 /srv/www/htdocs/nextcloud-release/3rdparty/symfony/console/Application.php(273): Symfony\Component\Console\Application->doRunCommand()
#9 /srv/www/htdocs/nextcloud-release/3rdparty/symfony/console/Application.php(149): Symfony\Component\Console\Application->doRun()
#10 /srv/www/htdocs/nextcloud-release/lib/private/Console/Application.php(209): Symfony\Component\Console\Application->run()
#11 /srv/www/htdocs/nextcloud-release/console.php(99): OC\Console\Application->run()
#12 /srv/www/htdocs/nextcloud-release/occ(11): require_once('/srv/www/htdocs...')
#13 {main}

@PVince81
Copy link
Member

PVince81 commented Feb 2, 2022

actually, my tests all were done on 22.2.3 patched with this PR

CarlSchwan
CarlSchwan reviewed Feb 2, 2022
View reviewed changes
@icewind1991 icewind1991 force-pushed the appdata-lite-filesystem branch from 7644f8e to 9371ab2 Compare February 3, 2022 14:35
@icewind1991
Copy link
Member Author

@PVince81 scan-app-data is fixed

@icewind1991
Copy link
Member Author

This is ready to merge from my pov

PVince81
PVince81 approved these changes Feb 3, 2022
View reviewed changes
Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

👍

@PVince81 PVince81 added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Feb 3, 2022
CarlSchwan
CarlSchwan approved these changes Feb 3, 2022
View reviewed changes
Copy link
Member

@CarlSchwan CarlSchwan left a comment

Choose a reason for hiding this comment

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

a minor thing to fix but other than that it's good to go 🥇

@PVince81
Copy link
Member

PVince81 commented Feb 4, 2022

@icewind1991 also DCO

@icewind1991 icewind1991 force-pushed the appdata-lite-filesystem branch from 57eb8e7 to b2efee0 Compare February 9, 2022 13:49
@icewind1991
only setup part of the filesystem for appdata requests
0217949 
Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991
pass user object during fs init
1ab58ef 
Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991
fix fs cache test user setup
2673775 
Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991
dont mark fs as setup when no user is active
b799fd4 
Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991
handle setupFS with null user
de26000 
Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991 icewind1991 force-pushed the appdata-lite-filesystem branch from b2efee0 to de26000 Compare February 9, 2022 15:01
@PVince81 PVince81 merged commit e80e0d5 into master Feb 10, 2022
@PVince81 PVince81 deleted the appdata-lite-filesystem branch February 10, 2022 08:58
@PVince81
Copy link
Member

/backport to stable23

@PVince81
Copy link
Member

/backport to stable22

@PVince81
Copy link
Member

need to be careful with the backports, probably wait for more testing to happen on master first

@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Feb 10, 2022
Merged
@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Feb 10, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nickvergessen nickvergessen nickvergessen left review comments

@blizzz blizzz blizzz left review comments

@PVince81 PVince81 PVince81 approved these changes

@CarlSchwan CarlSchwan CarlSchwan approved these changes

Assignees

No one assigned

Labels

3. to review Waiting for reviews performance 🚀

Projects

None yet

Milestone

Nextcloud 24

Development

Successfully merging this pull request may close these issues.

6 participants

@icewind1991 @PVince81 @nickvergessen @blizzz @CarlSchwan