Skip to content

Fix sync loop for too long path - #316

Open
harminius wants to merge 13 commits into
masterfrom
fix/sync_loop_for_too_long_path
Open

Fix sync loop for too long path#316
harminius wants to merge 13 commits into
masterfrom
fix/sync_loop_for_too_long_path

Conversation

@harminius

@harminius harminius commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Fixes MerginMaps/qgis-plugin#932

Use \\?\-prefixed paths for geodiff calls and other file I/O.

Debugging info:
Pull works as py-client saves the diffs to tmp dir
However, there is always a check if there's something to push after the pull.

Push treats every gpkg with editing mode enabled as well as with original vs. current gpkg checksum mismatch (which is quite common) as potentially updated, and we rely on geodiff to calculate the real changes. Geodiff creates an empty diff file even when there is no change.
Geodiff's create_changeset() fails because it's not possible to create a file due to the Windows path length limit. It then pushes the whole file. Then we want to apply the push changes to the basefile. So geodiff removes the original geopackage and copies the current copy, which apparently changes the checksum, and we're caught in the loop. ➿

Site Feature
merginproject.py get_push_changes push
merginproject.py get_local_delta pull, local changes detection
client_pull.py get_download_diff_files pull, v1 fallback
client_pull.py pull_project_async v2 loop pull, v2
client_pull.py download_diffs_async get_file_diff / version viewer / reports
This is a minimal fix for the issue. There are other enhancements/issue mitigations:
  1. Shorten diff name
    Store diff files locally under a short, flat name, decoupled from the file's path in the project — e.g. .mergin/diffs/ instead of .mergin/-diff-. Fixes works esp. for files nested deep in the subfolders. This does not require server changes - the server doesn't care what we call the local file.
Path Length
Live file <rel> P + R + 1
Basefile .mergin<rel> — already mandatory today P + R + 9
Diff today .mergin<rel>-diff- P + R + 51
Diff flattened .mergin\diffs<uuid> P + 51 (constant)
Same, 16-char id P + 31 (constant)
  1. Safeguard for any infinite sync loop
    sync_project_generator's while has_changes has no iteration limit. Track, per file, how many consecutive iterations it was pushed as a full upload with no diff; if the same versioned file does this N times in a row (e.g. 2–3), raise a ClientError naming the file instead of continuing. This is cause-agnostic - it also protects against any other future reason a changeset can't be computed.
  1. Long path error workaround
    Use \\?\-prefixed paths for geodiff calls and other file I/O.

4. Use a temp folder for push diffs, as it is used for pull diffs already.

@harminius harminius changed the title Fix sync loop for too long path WIP: Fix sync loop for too long path Aug 11, 2026
@harminius

Copy link
Copy Markdown
Contributor Author

client-log.txt

@harminius

Copy link
Copy Markdown
Contributor Author

db paths look fine
Screenshot from 2026-08-17 21-32-34

@harminius harminius changed the title WIP: Fix sync loop for too long path Fix sync loop for too long path Aug 17, 2026
@coveralls

coveralls commented Aug 17, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 32980936115

Coverage increased (+0.07%) to 83.452%

Details

  • Coverage increased (+0.07%) from the base build.
  • Patch coverage: 9 uncovered changes across 4 files (103 of 112 lines covered, 91.96%).
  • 1 coverage regression across 1 file.

Uncovered Changes

File Changed Covered %
mergin/utils.py 15 10 66.67%
mergin/client_pull.py 12 10 83.33%
mergin/fs.py 22 21 95.45%
mergin/merginproject.py 52 51 98.08%
Total (7 files) 112 103 91.96%

Coverage Regressions

1 previously-covered line in 1 file lost coverage.

File Lines Losing Coverage Coverage
mergin/client_pull.py 1 82.75%

Coverage Stats

Coverage Status
Relevant Lines: 5735
Covered Lines: 4786
Line Coverage: 83.45%
Coverage Strength: 0.83 hits per line

💛 - Coveralls

Comment thread mergin/utils.py
prefix = backslash + backslash + "?" + backslash
if path.startswith(prefix):
return path
return prefix + os.path.abspath(path)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So this is only for local handling of files but it will never end up in relative paths for files in DB (we need this to work for all clients)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes, rel_path() handles the prefixed path correctly and removes it

Comment thread mergin/client.py Outdated
diffs = []
for v in versions_to_fetch[1:]:
diffs.append(mp.fpath_cache(file_history["history"][v]["diff"]["path"], v))
diffs.append(long_path(mp.fpath_cache(file_history["history"][v]["diff"]["path"], v)))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Using some patch function for a lot of os.paths hanging around in the code seems fragile to me. It can easily slip and we might forgot to use in future. I think we need to come up with some generic handler, single place which will make sure all paths are correct in whole code base.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Introduced wrapper to geodiff in bcc7ae6
and put all file system calls in one place 2051d4f

@harminius
harminius requested a review from varmar05 August 26, 2026 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sync loop path to diff files in .mergin folder exceeds 260 characters

3 participants