jabref icon indicating copy to clipboard operation
jabref copied to clipboard

Use git for backup

Open khola22 opened this issue 1 year ago • 17 comments

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.

Screenshot 2024-12-05 at 23 04 21

WhatsApp Image 2024-12-01 at 13 37 08

Screenshot 2024-12-05 at 23 04 39

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 : https://github.com/JabRef/jabref/issues/2961#top

Mandatory checks

  • [x] I own the copyright of the code submitted and I licence it under the MIT license
  • [x] Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
  • [x] Tests created for changes (if applicable)
  • [x] Manually tested changed features in running JabRef (always required)
  • [x] Screenshots added in PR description (for UI changes)
  • [x] Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • [x] Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

khola22 avatar Dec 02 '24 00:12 khola22

Please use the PR title and the description to elaborate what this PR is supposed to solve.

If it is a work in progress, mark it as draft.

subhramit avatar Dec 02 '24 07:12 subhramit

Hello ! I updated the description, I hope it is clearer now.

khola22 avatar Dec 02 '24 08:12 khola22

hello @subhramit , we were trying to resolve the conflicts with submodules as said in the link, the first approache didn't work, but for the second one, we have an issue :

  • (base) khaoula@Khaoulas-MacBook-Air JabRef % cd src/main/resources/csl-locales
  • (base) khaoula@Khaoulas-MacBook-Air csl-locales % git checkout 96d704d error: pathspec '96d704d' did not match any file(s) known to git
  • (base) khaoula@Khaoulas-MacBook-Air csl-locales % git checkout 96d704d error: pathspec '96d704d' did not match any file(s) known to git
  • (base) khaoula@Khaoulas-MacBook-Air csl-locales % git status HEAD detached at 8bc2af1 nothing to commit, working tree clean

khola22 avatar Dec 05 '24 00:12 khola22

Update : we fixed the conflicts relative to submodules.

khola22 avatar Dec 05 '24 10:12 khola22

By the way, really good work guys.

Please also take a look at the failing tests and try to fix them. https://devdocs.jabref.org/code-howtos/faq.html will help you.

subhramit avatar Dec 05 '24 12:12 subhramit

Hey team, good job fixing the tests. The maintainers are currently a bit busy, and will get to your PR as soon as they find time.

subhramit avatar Dec 11 '24 22:12 subhramit

Hello @subhramit , We wanted to let you know that we have carefully reviewed all your comments and made the necessary adjustments. Thank you for taking the time to provide your feedback—it is greatly appreciated.

khola22 avatar Dec 13 '24 21:12 khola22

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

subhramit avatar Jan 13 '25 06:01 subhramit

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

Linking a pull request to an issue using a keyword

You can link a pull request to an issue by using a supported keyword in the pull request's description or in a commit message. The pull request must be on the default branch.

  • close
  • closes
  • closed
  • fix
  • fixes
  • fixed
  • resolve
  • resolves
  • resolved

If you use a keyword to reference a pull request comment in another pull request, the pull requests will be linked. Merging the referencing pull request also closes the referenced pull request.

The syntax for closing keywords depends on whether the issue is in the same repository as the pull request.

Example:

  • Fixes #xyz links pull-request to issue. Merging the PR will close the Issue.
  • Fixes https://github.com/JabRef/jabref/issues/xyz links pull-request to issue. Merging the PR will close the Issue.
  • Fixes https://github.com/Koppor/jabref/issues/xyz links pull-request to issue. Merging the PR will close the Issue.
  • Fixes [#xyz](https://github.com/JabRef/jabref/issues/xyz) links pull-request to issue. Merging the PR will NOT close the Issue.

koppor avatar Jan 27 '25 21:01 koppor

Submodule config got messed up:

   Submodule 'abbrv.jabref.org' (https://github.com/JabRef/abbrv.jabref.org.git) registered for path 'buildres/abbrv.jabref.org'
  Error: fatal: No url found for submodule path 'jabref' in .gitmodules

I try to find out how to fix - will be difficult.

koppor avatar Jan 27 '25 21:01 koppor

Submodule config got messed up:

   Submodule 'abbrv.jabref.org' (https://github.com/JabRef/abbrv.jabref.org.git) registered for path 'buildres/abbrv.jabref.org'
  Error: fatal: No url found for submodule path 'jabref' in .gitmodules

I try to find out how to fix - will be difficult.

 git rm --cached jabref

Hint https://www.deployhq.com/support/common-repository-errors/no-url-found-for-submodule (found via Google)

koppor avatar Jan 27 '25 21:01 koppor

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.

koppor avatar Jan 27 '25 22:01 koppor

@koppor Thanks for diving into the code! I've just made the necessary changes to address the non-code conventions you mentioned. Let us know if there's anything else we can improve!

nawalchahboune avatar Feb 11 '25 20:02 nawalchahboune

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.

github-actions[bot] avatar Mar 12 '25 22:03 github-actions[bot]

I tried to merge main into this.

The diff shows

image

image

  • startBackupTask( was moved around in the class, thus a "normal" diff is useless.

Some "useless JavaDoc entries (they are derived from the name directly not adding any new informatoin)

     * @param backupDir the backup directory
     * @param dbfile the database file
     * @throws IOException     if an I/O error occurs
     * @throws GitAPIException if a Git API error occurs
        // Commit the staged changes
        RevCommit commit = git.commit()

The diff for CHANGELOG.md is also larger than it should

Following is bad style

throw new RuntimeException("Failed to save the database", e);

I think, we had the boolean at saveAs for successful/non-successfull execution

Normalization of BibTeX is not done using the LineEndingNormalizer or the bibtex saver itself (which normalizes internally), but differently.


Seeing all this, I think, the core team needs to take over

koppor avatar Mar 12 '25 22:03 koppor

Yes the feature is really promising and the team around the op did already a great job, but now it comes to leveraging this huge project to make it mergable. We can maybe put this in our milestone for the next beta.

calixtus avatar Mar 13 '25 12:03 calixtus

After merge

  • [ ] Answer at https://discourse.jabref.org/t/references-suddenly-wiped-urgent/4407 and close the topic

koppor avatar Jun 02 '25 13:06 koppor