Use git for backup #12252
Use git for backup #12252
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Hey team, good job fixing the tests. |
|
Hello @subhramit , |
| entries1.add(new BibEntry("this")); | ||
| entries1.add(new BibEntry("is")); |
There was a problem hiding this comment.
Use different entry types (e.g. Article). Also add fields to the entries using .withField(...)
Example -
.gitmodules
Outdated
| @@ -13,3 +13,6 @@ | |||
| url = https://github.com/citation-style-language/locales.git | |||
| ignore = all | |||
| shallow = true | |||
| [submodule "jabref"] | |||
There was a problem hiding this comment.
and also fix the other submodule issues.
You probably have to first sync your main branch with the latest changes from upstream and then your branch
There was a problem hiding this comment.
Hello ! I am sorry for the late reply. I applied what is written here https://devdocs.jabref.org/code-howtos/faq.html#submodules to fix the submodules, it worked correctly because I no more have local changes when I execute : git checkout main -- src/main/resources/csl-styles, still, the problem with the submodule csl-styles doesn't seem to be resolved. And can you tell me what changes am I supposed to adress in the CHANGELOG (we already added the suggested edits from the last month, but there is still some changes required by the PR).
|
@khola22 we will be making sure your feature is a part of the 6.0-beta release. Thanks for so much patience, it's just that the maintainers are a bit busy with a couple of things (and other urgent issues) at the moment. We'll provide more feedback as soon as we can. |
# Conflicts: # src/main/resources/csl-locales # src/main/resources/csl-styles
There was a problem hiding this comment.
Your code currently does not meet JabRef's code guidelines.
We use Checkstyle to identify issues.
Please carefully follow the setup guide for the codestyle.
Afterwards, please run checkstyle locally and fix the issues.
In case of issues with the import order, double check that you activated Auto Import.
You can trigger fixing imports by pressing Ctrl+Alt+O to trigger Optimize Imports.
|
To ease organizational workflows I have linked the pull-request to the issue with syntax as described in https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue
Example:
|
|
Submodule config got messed up: I try to find out how to fix - will be difficult. |
Hint https://www.deployhq.com/support/common-repository-errors/no-url-found-for-submodule (found via Google) |
There was a problem hiding this comment.
I started to dive into the code, but some basic non-code conventions distracted me. Please try to fix them. They might seem to be trivial for you, but I would need several minutes for it, but I currently don't have time to do these kind of tasks. I need time for feedback on a higher level.
| LOGGER.info("Database file uniquely copied to backup directory: {}", backupFilePath); | ||
| } | ||
|
|
||
| // A method |
| LOGGER.info("Successfully rewrote the file at path: {}", dbFile); | ||
| } | ||
|
|
||
| // Helper method to normalize BibTeX content |
There was a problem hiding this comment.
Convert to JAvaDoc - ALL comments to Methods have to be JavaDoc
The WHY is missing the comment -why is this needed?
| * @param backupDir the backup directory | ||
| * @param objectId the commit ID to restore from | ||
| */ | ||
|
|
There was a problem hiding this comment.
JavaDoc and method begin should be kept together - please remove all empty lines between JavaDoc and method.
| if (needsBackup) { | ||
| LOGGER.info("Backup needed, because needsBackup is :{}", needsBackup); | ||
| } else { | ||
| LOGGER.info("Backup needed, because needsCommit is :{}", needsCommit); |
There was a problem hiding this comment.
indent is wrong. I wonder why our checkstyle did not comlain here.
| } | ||
|
|
||
| if (needsBackup) { | ||
| LOGGER.info("Backup needed, because needsBackup is :{}", needsBackup); |
There was a problem hiding this comment.
| LOGGER.info("Backup needed, because needsBackup is :{}", needsBackup); | |
| LOGGER.info("Backup needed, because needsBackup is: {}", needsBackup); |
Please use Amercian positioning of spaces and colon: First comes the colon, then the space.
| String content = Localization.lang("It looks like JabRef did not shut down cleanly last time the file was used.") + "\n\n" + | ||
| Localization.lang("Do you want to recover the library from a backup file?"); |
There was a problem hiding this comment.
Merge this into one string!
(Even though in the original, newlines were also split like this, but this is wrong)
|
Note that I renamed "BackupManagerGit" to "BackupManager" as the "old" BackupManager is gone. No need to create a new class. Keeping the base class name also simplifies reviewing. |
Fiixes #2961
This PR introduces several key updates and improvements aimed at enhancing the backup management, change detection, and UI functionalities.
Backup Management with Git Integration:
A new BackupManagerGit class has been implemented to handle backups using Git. This allows .bib files to be automatically copied to a dedicated jabref/backups directory every 19 seconds, ensuring that a comprehensive commit history is maintained via Git. This approach provides a centralized and version-controlled backup mechanism, independent of the file location.
Change Detection Logic:
Change detection is now triggered when .bib files are overwritten during explicit user actions such as Save, Save All, or the save prompts when exiting the application. While listeners continue to track edits, their strict conditions occasionally limit their ability to dynamically detect changes. As a result, we face two challenges:
Lack of autosave functionality without user interaction.
Potential failure to record unsaved changes if the application closes unexpectedly.
UI Enhancements:
The UI has been updated to provide functionality for saving, committing, and discarding changes, along with accurate commit details retrieval. While most features are working as expected, challenges remain with the checkout functionality, especially when reverting commits using JGit. We welcome feedback on this part of the implementation.
Date of backup refers to the date of the commit.
Code Refactoring and Adaptation:
We’ve refactored various components to integrate with BackupManagerGit, replacing the older BackupManager logic. Existing classes like CoarseChangeFilter have been retained without modification, as our focus was on implementing the new Git-based backup mechanism.
PS 1: We have retained the BackupManager class for reference, allowing us to review its contents should further clarification of the backup logic be necessary. However, we have ensured that this class is no longer used outside of the specific tests previously dedicated to it, and we have confirmed that BackupManagerGit is the primary class in operation. Despite these precautions, we have encountered an issue with the legacy backup tracking method. Specifically, we have observed occasional, unexpected creation of .bak files within the backup directory. This anomaly has occurred only once during testing. While we suspect the issue may be linked to the UI components of the code, we have yet to pinpoint the precise cause.
PS 2: A potential issue arose concerning the handling of two .bib files with identical names. Copying such files into the repository would result in one file overwriting the other, which is problematic. We are currently investigating a solution to this before finalizing the PR for review.
PS 3 : We didn't implement squashing older backup commits.
Update 1: The issue identified in PS 2 has been addressed by implementing UUID-generated unique file names. However, when a .bib file is relocated, a new UUID is generated, which results in a new file being recorded in the Git repository. To resolve this, we propose the following solutions: 1/ Move the .uuid file along with the .bib file when the move is performed through JabRef. 2/ Utilize a file system watcher to track and handle file movements performed outside of JabRef.
Updates 2 : Restore button works.
We have not yet pursued further investigation into these solutions.
The title of the isuue : Enhanced backup restore dialog #2961
The link of the issue : #2961 (comment)
Mandatory checks
CHANGELOG.mddescribed in a way that is understandable for the average user (if change is visible to the user)