abraunegg / onedrive

OneDrive Client for Linux
https://abraunegg.github.io
GNU General Public License v3.0
9.88k stars 855 forks source link

sync_list: including folders it shouldn't when their name is included in another folder's name #1388

Closed SylvainLaugie closed 3 years ago

SylvainLaugie commented 3 years ago

Bug Report Details

When using sync_list, if you have two folders in root of OneDrive, the name of the first being the beginning of the second, and only the second is in sync_list:

Then the first folder "Myfolder" is included, but it shouldn't be.

Files and subfolders of "Myfolder" are not included (which is normal), but the folder itself is locally created during sync. Moreover, if this folder contains a lot of files (like thousands), sync takes way more time because it looks into the folder on OneDrive.

Application and Operating System Details:

To Reproduce

1) Have the following folders & files in your OneDrive:

Myfolder/
Myfolder/myfile
MyfolderSuper/
MyfolderSuper/MysubfolderSuper1/myfile1.txt
MyfolderSuper/MysubfolderSuper2/myfile2.txt

2) Have the sync_list file with this content:

MyfolderSuper/*

3) Do the first sync (logs taken at the same time here):

/usr/local/bin/onedrive --synchronize --resync --download-only --verbose --verbose 2>&1 > debug.log

4) Result: Myfolder is created (empty) even if not in sync_list:

root@hostname:~/OneDrive# ll -R
.:
total 8
drwx------ 2 root root 4096 Apr  6 23:56 Myfolder
drwx------ 4 root root 4096 Apr  6 23:56 MyfolderSuper

./Myfolder:
total 0

./MyfolderSuper:
total 8
drwx------ 2 root root 4096 Apr  6 23:56 MysubfolderSuper1
drwx------ 2 root root 4096 Apr  6 23:56 MysubfolderSuper2

./MyfolderSuper/MysubfolderSuper1:
total 4
-rw------- 1 root root 3 Apr  6 23:08 myfile1.txt

./MyfolderSuper/MysubfolderSuper2:
total 4
-rw------- 1 root root 3 Apr  6 23:09 myfile2.txt

Interesting parts of the logs:

debug.log:588: Myfolder is a direct match, but it shouldn't be:

...
[DEBUG] id == defaultRootId                                  = true
[DEBUG] isItemRoot(item)                                     = false
[DEBUG] item['name'].str == 'root'                           = false
[DEBUG] singleDirectoryScope                                 = false
...
[DEBUG] thisItemParentPath                                   = /drive/root:
[DEBUG] thisItemFullPath                                     = /drive/root:/Myfolder
...
[DEBUG] Evaluation against 'sync_list' for this path: Myfolder
[DEBUG] [S]exclude        = false
[DEBUG] [S]excludeMatched = false
[DEBUG] Evaluation against 'sync_list' entry: MyfolderSuper/*
[DEBUG] Evaluation against 'sync_list' result: direct match
[DEBUG] [F]exclude        = false
[DEBUG] [F]excludeMatched = false
[DEBUG] Evaluation against 'sync_list' final result: included for sync
[DEBUG] OneDrive change is a new local item
Creating local directory: Myfolder
[DEBUG] Issue #658 handling
[DEBUG] Setting oneDriveFullScanTrigger = true due to new folder creation request in a location that is now in-scope which may have previously out of scope
[DEBUG] Requested path does not exist, creating directory structure: Myfolder
[DEBUG] Setting directory permissions for: Myfolder
[DEBUG] Inserting item details to local database
...

I understand this comes from https://github.com/abraunegg/onedrive/blob/master/src/selective.d#L279 : "Myfolder" is both the path and the common prefix, so it is matched.

abraunegg commented 3 years ago

@SylvainLaugie Was this issue present before the v2.4.11 release? This is not a regression due to the v2.4.11 release:

./onedrive --confdir '~/.config/onedrive-personal/' --synchronize --verbose --resync
Using 'user' Config Dir: /home/alex/.config/onedrive-personal/
Using 'system' Config Dir: 
Configuration file successfully loaded
Deleting the saved status ...
Initializing the OneDrive API ...
Configuring Global Azure AD Endpoints
Opening the item database ...
All operations will be performed in: /home/alex/OneDrivePersonal
Application version: v2.4.9-13-ge163d57    <-- release v2.4.10 commit
Account Type: personal
Default Drive ID: 66d53be8a5056eca
Default Root ID: 66D53BE8A5056ECA!101
Remaining Free Space: 5368703366
Fetching details for OneDrive Root
OneDrive Root does not exist in the database. We need to add it.
Added OneDrive Root to the local database
Initializing the Synchronization Engine ...
Syncing changes from OneDrive ...
Applying changes of Path ID: 66D53BE8A5056ECA!101
Updated Remaining Free Space: 5368703366
Processing 9 OneDrive items to ensure consistent local state due to sync_list being used
Creating local directory: Myfolder
Creating local directory: MyfolderSuper
Skipping item - excluded by sync_list config: Myfolder/myfile
Creating local directory: MyfolderSuper/MysubfolderSuper2
Creating local directory: MyfolderSuper/MysubfolderSuper1
Downloading file MyfolderSuper/MysubfolderSuper2/myfile2.txt ... done.
Downloading file MyfolderSuper/MysubfolderSuper1/myfile1.txt ... done.
Processing 1 OneDrive items to ensure consistent local state due to a full scan being triggered by actions on OneDrive
Uploading differences of ~/OneDrivePersonal
Processing .
The directory has not changed
Processing Myfolder
The directory has not changed
Processing MyfolderSuper
The directory has not changed
Processing MyfolderSuper/MysubfolderSuper2
The directory has not changed
Processing MyfolderSuper/MysubfolderSuper2/myfile2.txt
The file has not changed
Processing MyfolderSuper/MysubfolderSuper1
The directory has not changed
Processing MyfolderSuper/MysubfolderSuper1/myfile1.txt
The file has not changed
Uploading new items of ~/OneDrivePersonal
Applying changes of Path ID: 66D53BE8A5056ECA!101
Updated Remaining Free Space: 5368703366
Processing 1 OneDrive items to ensure consistent local state due to a full scan being requested
abraunegg commented 3 years ago

@SylvainLaugie

Please can you test the following PR to resolve this issue:

git clone https://github.com/abraunegg/onedrive.git
cd onedrive
git fetch origin pull/1390/head:pr1390
git checkout pr1390
./configure; make clean; make;

To run the PR, you need to run the client from the PR build directory:

./onedrive <any options needed>

When running the PR, your version should be: onedrive v2.4.11-2-g1f658e4 or greater

SylvainLaugie commented 3 years ago

@abraunegg

I tested your PR, it works well if you have the /* just after first folder in sync_list: MyfolderSuper/*.

However, it breaks things when you have a longer path, with subfolders before the /* : MyfolderSuper/MysubfolderSuper1/*

Debug output:

[DEBUG] Evaluation against 'sync_list' for this path: MyfolderSuper
[DEBUG] [S]exclude        = false
[DEBUG] [S]excludeMatched = false
[DEBUG] Evaluation against 'sync_list' entry: MyfolderSuper/MysubfolderSuper1/*
[DEBUG] slaugie commonPrefix: MyfolderSuper
[DEBUG] slaugie potential exact match
[DEBUG] slaugie strippedAllowedPath: MyfolderSuper/MysubfolderSuper1
[DEBUG] [F]exclude        = false
[DEBUG] [F]excludeMatched = false
[DEBUG] Evaluation against 'sync_list' final result: EXCLUDED

MyfolderSuper and all subfolders/files which should be synced are skipped.

[DEBUG] thisItemFullPath                                     = /drive/root:/MyfolderSuper/MysubfolderSuper1/myfile1.txt
[DEBUG] 'item id' matches search 'id'                        = false
[DEBUG] 'parentReference id' matches search 'id'             = false
[DEBUG] 'thisItemParentPath' contains 'syncFolderChildPath'  = true
[DEBUG] 'thisItemParentPath' contains search 'id'            = false
[DEBUG] Change matches search criteria to apply
[DEBUG] lastModifiedDateTime (OneDrive item): 2021-Apr-06 21:08:49.59Z
[DEBUG] Flagging as unwanted: find(item.parentId).length != 0
[DEBUG] Skipping OneDrive change as this is determined to be unwanted
abraunegg commented 3 years ago

@SylvainLaugie Interesting .. as the further matching should be via sub-path ... thanks for the feedback, will investigate further.

abraunegg commented 3 years ago

@SylvainLaugie Please can you test the updated PR (client version onedrive v2.4.11-5-g2ad2d83). Based on my testing:

sync_list entry: MyfolderSuper/*

Using 'user' Config Dir: /home/alex/.config/onedrive-personal/
Using 'system' Config Dir: 
Configuration file successfully loaded
Deleting the saved status ...
Initializing the OneDrive API ...
Configuring Global Azure AD Endpoints
Opening the item database ...
All operations will be performed in: /home/alex/OneDrivePersonal
Application version: v2.4.11-3-g0eea13d
Account Type: personal
Default Drive ID: 66d53be8a5056eca
Default Root ID: 66D53BE8A5056ECA!101
Remaining Free Space: 5368703366
Fetching details for OneDrive Root
OneDrive Root does not exist in the database. We need to add it.
Added OneDrive Root to the local database
Initializing the Synchronization Engine ...
Syncing changes from OneDrive ...
Applying changes of Path ID: 66D53BE8A5056ECA!101
Updated Remaining Free Space: 5368703366
Processing 9 OneDrive items to ensure consistent local state due to sync_list being used
Skipping item - excluded by sync_list config: Myfolder
Creating local directory: MyfolderSuper
Creating local directory: MyfolderSuper/MysubfolderSuper2
Creating local directory: MyfolderSuper/MysubfolderSuper1
Downloading file MyfolderSuper/MysubfolderSuper2/myfile2.txt ... done.
Downloading file MyfolderSuper/MysubfolderSuper1/myfile1.txt ... done.
Processing 1 OneDrive items to ensure consistent local state due to a full scan being triggered by actions on OneDrive
Uploading differences of ~/OneDrivePersonal
Processing .
The directory has not changed
Processing MyfolderSuper
The directory has not changed
Processing MyfolderSuper/MysubfolderSuper2
The directory has not changed
Processing MyfolderSuper/MysubfolderSuper2/myfile2.txt
The file has not changed
Processing MyfolderSuper/MysubfolderSuper1
The directory has not changed
Processing MyfolderSuper/MysubfolderSuper1/myfile1.txt
The file has not changed
Uploading new items of ~/OneDrivePersonal
Applying changes of Path ID: 66D53BE8A5056ECA!101
Updated Remaining Free Space: 5368703366
Processing 1 OneDrive items to ensure consistent local state due to a full scan being requested

sync_list entry: MyfolderSuper/MysubfolderSuper1/*

Using 'user' Config Dir: /home/alex/.config/onedrive-personal/
Using 'system' Config Dir: 
Configuration file successfully loaded
Deleting the saved status ...
Initializing the OneDrive API ...
Configuring Global Azure AD Endpoints
Opening the item database ...
All operations will be performed in: /home/alex/OneDrivePersonal
Application version: v2.4.11-3-g0eea13d
Account Type: personal
Default Drive ID: 66d53be8a5056eca
Default Root ID: 66D53BE8A5056ECA!101
Remaining Free Space: 5368703366
Fetching details for OneDrive Root
OneDrive Root does not exist in the database. We need to add it.
Added OneDrive Root to the local database
Initializing the Synchronization Engine ...
Syncing changes from OneDrive ...
Applying changes of Path ID: 66D53BE8A5056ECA!101
Updated Remaining Free Space: 5368703366
Processing 9 OneDrive items to ensure consistent local state due to sync_list being used
Skipping item - excluded by sync_list config: Myfolder
Creating local directory: MyfolderSuper
Skipping item - excluded by sync_list config: MyfolderSuper/MysubfolderSuper2
Creating local directory: MyfolderSuper/MysubfolderSuper1
Downloading file MyfolderSuper/MysubfolderSuper1/myfile1.txt ... done.
Processing 1 OneDrive items to ensure consistent local state due to a full scan being triggered by actions on OneDrive
Uploading differences of ~/OneDrivePersonal
Processing .
The directory has not changed
Processing MyfolderSuper
The directory has not changed
Processing MyfolderSuper/MysubfolderSuper1
The directory has not changed
Processing MyfolderSuper/MysubfolderSuper1/myfile1.txt
The file has not changed
Uploading new items of ~/OneDrivePersonal
Applying changes of Path ID: 66D53BE8A5056ECA!101
Updated Remaining Free Space: 5368703366
Processing 1 OneDrive items to ensure consistent local state due to a full scan being requested
SylvainLaugie commented 3 years ago

Hello,

This works for me, file is now included in the sync 👍

I tried several sync_list configurations to be sure but I didn't manage to break things (at least from because of this PR). I'll just leave a comment on the PR about one detail ni the code.

However, I did find something wrong when testing, but I checked and it was already present before (tested on v2.4.11). With the following sync_list:

-MyfolderSuper/*
MyfolderSuper/MysubfolderSuper1/*
MyfolderSuper/MysubfolderSuper2/myfile2.txt

Folder MysubfolderSuper1 is created but files in it are excluded.

This is not a problem for me (I don't use excludes) but do you want me to raise a new issue or is it OK?

abraunegg commented 3 years ago

@SylvainLaugie

Hello,

This works for me, file is now included in the sync 👍

I tried several sync_list configurations to be sure but I didn't manage to break things (at least from because of this PR). I'll just leave a comment on the PR about one detail ni the code.

However, I did find something wrong when testing, but I checked and it was already present before (tested on v2.4.11). With the following sync_list:

-MyfolderSuper/*
MyfolderSuper/MysubfolderSuper1/*
MyfolderSuper/MysubfolderSuper2/myfile2.txt

Folder MysubfolderSuper1 is created but files in it are excluded.

This is not a problem for me (I don't use excludes) but do you want me to raise a new issue or is it OK?

This is not an issue as your exclusion is a wildcard, thus you are wild carding all sub folders. With this configuration, if you only want to include MysubfolderSuper{1,2} then you do not need -MyfolderSuper/* in your 'sync_list' as it is already excluded.

SylvainLaugie commented 3 years ago

@abraunegg Ok clear, thanks !

github-actions[bot] commented 3 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.