MerginMaps / MerginMaps/qgis-plugin
diff files stay in .mergin folder after sync
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 44
- Forks
- 20
- Avg merge
- 3d 12h
- Merged PRs (30d)
- 5
Description
While testing this issue #777 we found out that diff files stay in the .mergin folder also after successful synchronization.
Background:
We use the client's method project_status(project_dir) to identify the local changes when syncing. This method calls get_push_changes() which calls geodiff's method create_changeset(). This method apparently creates a diff file in the .mergin folder. The core issue is, that there is no other geodiff's method, which would return the push changes without creating the diff file. The diff file is only needed when we want to send it to the server - during actual sync.
We call this method three ❗ times during the sync in the plugin.
- When the sync dialog is shown. We want to display the changes in the sync dialog window. See plugin's
project_statusmethod - At the beginning, when the Sync is triggered. We do some validation to determine if the user has the rights to push changes and to learn whether the QGIS project file has changed - to refresh the project. See
sync_projectmethod. - At the actual synchronization. When we want to apply the changes. See plugin's
push_start_internal()calling client'spush_project_async()
There are two issues:
I, The potentially unwanted / dangerous consequence is we work with several independently retrieved push changes, and we pretend they are the same. They mostly are, but there are some windows for foreign syncers, so that the changes displayed in the dialog (1) may not be the same like those used for pre-sync validation (2), which may differ from those that are actually applied on the server (3).
II, The second consequence how we found out this issue. There remain two unused diff files after the synchronization. Those diff files created in 1 and 2 are unused, the diff file created in 3 is used and cleaned in client's project_push_finalize()
Proposed solution
-
Quick and dirty - solves II:
Clean the unused diff files in 1 and 2 after they are created in plugin or add adryflag to client'sget_push_changesto cleanup the diffs. -
Proper
a, Introduce geodiff's method to return changeset only as a dictionary and do not create the diff file. Solves II.
b, Refactor sync to pass the changes that are calculated only once. Solves I and II.
Might be solved in v2 sync refactoring when we want to push all local changes regardless of the server's changes.
An approach also suggested here: https://github.com/MerginMaps/qgis-plugin/issues/777#issuecomment-3306280944
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the plugin's project_status, sync_project, and push_start_internal() paths, then trace the client's get_push_changes(), push_project_async(), and project_push_finalize() calls. Confirm which calls create diff files in .mergin and how cleanup currently works. Done means unused diff files are removed and the chosen approach addresses the repeated change calculation described here.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- tooling
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100