Skip to content

only setup part of the filesystem for appdata requests - #30942

Merged
PVince81 merged 5 commits into
masterfrom
appdata-lite-filesystem
Feb 10, 2022
Merged

only setup part of the filesystem for appdata requests#30942
PVince81 merged 5 commits into
masterfrom
appdata-lite-filesystem

Conversation

@icewind1991

@icewind1991icewind1991 commented Jan 31, 2022

Copy link
Copy Markdown
Member

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

@icewind1991icewind1991 added the 2. developing Work in progress label Jan 31, 2022
@icewind1991icewind1991 added this to the Nextcloud 24 milestone Jan 31, 2022
@icewind1991
icewind1991force-pushed the appdata-lite-filesystem branch 2 times, most recently from 3ba5f91 to 28662e3CompareJanuary 31, 2022 17:09
blizzz
blizzz reviewed Jan 31, 2022
View reviewed changes
Comment threadlib/public/Files/Config/IScopedMountProvider.php Outdated
@icewind1991
icewind1991force-pushed the appdata-lite-filesystem branch 2 times, most recently from 3be90b1 to 1a8db39CompareJanuary 31, 2022 17:50
nickvergessen
nickvergessen reviewed Jan 31, 2022
View reviewed changes
Comment threadlib/private/Files/Mount/Manager.php
nickvergessen
nickvergessen reviewed Feb 1, 2022
View reviewed changes
Comment threadlib/private/legacy/OC_Util.php Outdated
@icewind1991
icewind1991force-pushed the appdata-lite-filesystem branch 6 times, most recently from 0cefccd to 5808492CompareFebruary 1, 2022 18:05
@juliusknorrjuliusknorr mentioned this pull request Feb 2, 2022
Closed
@PVince81

PVince81 commented Feb 2, 2022•
edited
Loading

Copy link
Copy Markdown
Member

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

PVince81 commented Feb 2, 2022•
edited by icewind1991
Loading

Copy link
Copy Markdown
Member

@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

PVince81 commented Feb 2, 2022

Copy link
Copy Markdown
Member

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

CarlSchwan
CarlSchwan reviewed Feb 2, 2022
View reviewed changes
Comment threadlib/private/legacy/OC_Util.php Outdated
Comment threadapps/files_external/lib/Config/ConfigAdapter.php
@icewind1991
icewind1991force-pushed the appdata-lite-filesystem branch from 7644f8e to 9371ab2CompareFebruary 3, 2022 14:35
@icewind1991

icewind1991 commented Feb 3, 2022

Copy link
Copy Markdown
MemberAuthor

@PVince81scan-app-data is fixed

@icewind1991

icewind1991 commented Feb 3, 2022

Copy link
Copy Markdown
MemberAuthor

This is ready to merge from my pov

PVince81
PVince81 approved these changes Feb 3, 2022
View reviewed changes

@PVince81PVince81 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

@PVince81PVince81 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

@CarlSchwanCarlSchwan left a comment

Copy link
Copy Markdown
Member

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 🥇

Comment threadlib/private/Files/Filesystem.php Outdated
@PVince81

PVince81 commented Feb 4, 2022

Copy link
Copy Markdown
Member

@icewind1991 also DCO

@icewind1991
icewind1991force-pushed the appdata-lite-filesystem branch from 57eb8e7 to b2efee0CompareFebruary 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
icewind1991force-pushed the appdata-lite-filesystem branch from b2efee0 to de26000CompareFebruary 9, 2022 15:01
@PVince81
PVince81 merged commit e80e0d5 into masterFeb 10, 2022
@PVince81
PVince81 deleted the appdata-lite-filesystem branch February 10, 2022 08:58
@PVince81

PVince81 commented Feb 10, 2022

Copy link
Copy Markdown
Member

/backport to stable23

@PVince81

PVince81 commented Feb 10, 2022

Copy link
Copy Markdown
Member

/backport to stable22

@PVince81

PVince81 commented Feb 10, 2022

Copy link
Copy Markdown
Member

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

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

Reviewers

@nickvergessennickvergessennickvergessen left review comments
@blizzzblizzzblizzz left review comments
@PVince81PVince81PVince81 approved these changes
@CarlSchwanCarlSchwanCarlSchwan approved these changes

Assignees

No one assigned

Labels

3. to reviewWaiting for reviewsperformance 🚀

Projects

None yet

Milestone

Nextcloud 24

Development

Successfully merging this pull request may close these issues.

5 participants

@icewind1991@PVince81@nickvergessen@blizzz@CarlSchwan