Use git for backup #12252

khola22 · 2024-12-02T00:24:59Z

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

I own the copyright of the code submitted and I licence it under the MIT license
Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
Tests created for changes (if applicable)
Manually tested changed features in running JabRef (always required)
Screenshots added in PR description (for UI changes)
Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
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.

Removed BackupManagerJGit to do UI changes

…nch-1

…into branch-1" This reverts commit df21a4d, reversing changes made to 62a8beb.

…into branch-1" This reverts commit 0f9fcb9.

Siedlerchr · 2025-01-03T11:31:52Z

.gitmodules

@@ -13,3 +13,6 @@
  url = https://github.com/citation-style-language/locales.git
  ignore = all
  shallow = true
+[submodule "jabref"]


Revert this change

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

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

subhramit · 2025-01-13T06:14:49Z

@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

github-actions

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.

koppor · 2025-01-27T21:48:07Z

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 · 2025-01-27T21:50:24Z

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 · 2025-01-27T21:53:22Z

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

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.

koppor · 2025-01-27T22:01:13Z

src/main/java/org/jabref/gui/autosaveandbackup/BackupManager.java

+        LOGGER.info("Database file uniquely copied to backup directory: {}", backupFilePath);
+    }
+
+    // A method


This comment can be removed.

koppor · 2025-01-27T22:02:07Z

src/main/java/org/jabref/gui/autosaveandbackup/BackupManager.java

+        LOGGER.info("Successfully rewrote the file at path: {}", dbFile);
+    }
+
+    // Helper method to normalize BibTeX content


Convert to JAvaDoc - ALL comments to Methods have to be JavaDoc

The WHY is missing the comment -why is this needed?

koppor · 2025-01-27T22:02:44Z

src/main/java/org/jabref/gui/autosaveandbackup/BackupManager.java

+     * @param backupDir the backup directory
+     * @param objectId the commit ID to restore from
+     */
+


JavaDoc and method begin should be kept together - please remove all empty lines between JavaDoc and method.

koppor · 2025-01-27T22:03:01Z

src/main/java/org/jabref/gui/autosaveandbackup/BackupManager.java

+        if (needsBackup) {
+            LOGGER.info("Backup needed, because needsBackup is :{}", needsBackup);
+        } else {
+        LOGGER.info("Backup needed, because needsCommit is :{}", needsCommit);


indent is wrong. I wonder why our checkstyle did not comlain here.

koppor · 2025-01-27T22:03:30Z

src/main/java/org/jabref/gui/autosaveandbackup/BackupManager.java

-        }
+
+        if (needsBackup) {
+            LOGGER.info("Backup needed, because needsBackup is :{}", needsBackup);


Suggested change

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.

koppor · 2025-01-27T22:05:45Z

src/main/java/org/jabref/gui/backup/BackupChoiceDialog.java

+        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?");


Merge this into one string!

(Even though in the original, newlines were also split like this, but this is wrong)

koppor · 2025-01-27T22:08:10Z

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.

nawalchahboune · 2025-02-11T20:39:44Z

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

subhramit · 2025-02-11T21:24:14Z

src/main/java/org/jabref/gui/autosaveandbackup/BackupManager.java

+
+
+
+
+
+


Remove all except one empty line

subhramit · 2025-02-11T21:26:08Z

src/main/resources/csl-styles

Sync submodules (it's happening repeatedly so please read the "prevention section in the faq - https://devdocs.jabref.org/code-howtos/faq.html#prevention)

subhramit · 2025-02-11T21:27:27Z

src/main/java/org/jabref/gui/autosaveandbackup/BackupManager.java

+        // copyDatabaseFileToBackupDir(dbFile, backupDir);
+
+        // Ensure the Git repository exists
+        LOGGER.info("Ensuring the .git is initialized");


Reduce log noise. Maybe use Logger.debug

subhramit · 2025-02-11T21:28:23Z

src/main/java/org/jabref/gui/autosaveandbackup/BackupManager.java

+            LOGGER.info("Committed content: {}", committedContent);
+            LOGGER.info("Current content: {}", currentContent);


If you are done using this for logging, remove or use debug mode

subhramit · 2025-02-11T21:29:20Z

src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java

            if (action.isActionNecessary(result, dialogService, preferences)) {
+                LOGGER.info("Action is necessary");


Copy above review comments for all logger.infos

Maybe a bit late, but howtos for logging in JabRef are written at https://devdocs.jabref.org/code-howtos/logging.html

The important line is

[email protected] = debug

In this way, you can read all your log infos while developing

github-actions · 2025-03-12T22:50:27Z

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.

trag-bot · 2025-03-12T22:50:43Z

src/main/java/org/jabref/gui/LibraryTab.java

+                       .onSuccess(result -> {
+                           try {
+                               newTab.onDatabaseLoadingSucceed(result);
+                           } catch (Exception e) {


Catches general Exception instead of specific exceptions (GitAPIException and IOException). This violates the principle of catching specific exceptions only.

trag-bot · 2025-03-12T22:50:45Z

src/main/java/org/jabref/gui/LibraryTab.java

@@ -343,7 +344,7 @@ private void onDatabaseLoadingFailed(Exception ex) {
        dialogService.showErrorDialogAndWait(title, content, ex);
    }

-    private void setDatabaseContext(BibDatabaseContext bibDatabaseContext) {
+    private void setDatabaseContext(BibDatabaseContext bibDatabaseContext) throws GitAPIException, IOException {


Method signature changed to throw exceptions but JavaDoc wasn't updated to reflect these changes. When method changes, its documentation must be updated accordingly.

koppor · 2025-03-12T22:55:47Z

I tried to merge main into this.

The diff shows

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

calixtus · 2025-03-13T12:43:49Z

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.

nawalchahboune and others added 30 commits October 23, 2024 20:18

l'ajout de la classe BackUpManagerJGit

094252c

l'ajout de la classe BackUpManagerJGit

d0f6346

Merge remote-tracking branch 'origin/main'

7fd8f89

l'ajout de la classe BackUpManagerJGit

9ab37a2

Add the class JGIT

d8812ef

first git implementation, not tested

af50c7c

ajout de la methide backupGitDiffers

e72d408

Added Pop up to choose backup

d427e05

adding methods 06/11

0737f48

methods aded and changed : retrieveCommitDetails , retreiveCommits

f68236f

I start switching French comments to English

129d175

finish switching comments from French to English

b82682a

Added table

34a2889

Removed BackupManagerJGit to do UI changes

Navigation through dialogs implemented

b299e5f

some modif

d2b3619

some modif

d84fcab

fix some issues

54fa60a

adding date details

9a8e615

Merge branch 'UI' into branch-1

62a8beb

Merge branch 'branch-1' of https://github.com/khola22/jabref into bra…

df21a4d

…nch-1

Revert "Merge branch 'branch-1' of https://github.com/khola22/jabref …

0f9fcb9

…into branch-1" This reverts commit df21a4d, reversing changes made to 62a8beb.

Reapply "Merge branch 'branch-1' of https://github.com/khola22/jabref …

7b6ee3f

…into branch-1" This reverts commit 0f9fcb9.

minor fixes

b67b2d9

Minor Synthax fixes

3f9584e

supression de classe BackupManagerJGit

76b6f92

differenciation BackupManagerOld et BackupManager

fc94296

Corrected errors related to BackupManagerGit class

d6df47a

Start tests

9a07077

OpenDatabaseAction adapted

9c86a64

Start adapting SaveDatabaseAction.java and LibraryTab.java

fa42e86

nawalchahboune added 2 commits December 15, 2024 12:18

resolve initialization of bibEntries

a638da5

resolve comment on Initialization of bibEntries

fa21f0e

Siedlerchr reviewed Jan 3, 2025

View reviewed changes

“Khaoula added 3 commits January 7, 2025 10:07

reverse .gitmodules

fcdd63b

cst-style

9be1cd2

abbrv

6c70924

Merge branch 'main' into branch-1

1f9acbe

# Conflicts: # src/main/resources/csl-locales # src/main/resources/csl-styles

github-actions bot reviewed Jan 27, 2025

View reviewed changes

Remove wrong jabref submodule

b45d909

koppor added 2 commits January 27, 2025 23:00

Rename "BackupManagerGit" to "BackupManager"

e77e2a5

Remove occurences of BackupManagerGit

687d4fd

koppor requested changes Jan 27, 2025

View reviewed changes

koppor changed the title ~~Git Integration and UI Improvements~~ Use git for backup Feb 5, 2025

fix problems related to javadoc and indent

7477e94

subhramit reviewed Feb 11, 2025

View reviewed changes

koppor removed the [outdated] type: feature label Mar 12, 2025

Merge branch 'main' into branch-1

b882f21

trag-bot bot reviewed Mar 12, 2025

View reviewed changes

subhramit added this to the 6.0-beta milestone Mar 13, 2025

koppor marked this pull request as draft March 19, 2025 20:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use git for backup #12252

Use git for backup #12252

khola22 commented Dec 2, 2024 •

edited by koppor

Loading

Siedlerchr Jan 3, 2025

Siedlerchr Jan 3, 2025

khola22 Jan 11, 2025

subhramit commented Jan 13, 2025 •

edited

Loading

github-actions bot left a comment

koppor commented Jan 27, 2025

Linking a pull request to an issue using a keyword

koppor commented Jan 27, 2025

koppor commented Jan 27, 2025

koppor left a comment

koppor Jan 27, 2025

koppor Jan 27, 2025

koppor Jan 27, 2025

koppor Jan 27, 2025

koppor Jan 27, 2025

koppor Jan 27, 2025

koppor commented Jan 27, 2025

nawalchahboune commented Feb 11, 2025

subhramit Feb 11, 2025

subhramit Feb 11, 2025

subhramit Feb 11, 2025

subhramit Feb 11, 2025

subhramit Feb 11, 2025

koppor Feb 16, 2025

koppor Feb 16, 2025

github-actions bot commented Mar 12, 2025

trag-bot bot Mar 12, 2025

trag-bot bot Mar 12, 2025

koppor commented Mar 12, 2025

calixtus commented Mar 13, 2025

	LOGGER.info("Backup needed, because needsBackup is :{}", needsBackup);
	LOGGER.info("Backup needed, because needsBackup is: {}", needsBackup);

		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?");

		LOGGER.info("Committed content: {}", committedContent);
		LOGGER.info("Current content: {}", currentContent);

		if (action.isActionNecessary(result, dialogService, preferences)) {
		LOGGER.info("Action is necessary");

Use git for backup #12252

Are you sure you want to change the base?

Use git for backup #12252

Conversation

khola22 commented Dec 2, 2024 • edited by koppor Loading

Mandatory checks

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

subhramit commented Jan 13, 2025 • edited Loading

github-actions bot left a comment

Choose a reason for hiding this comment

koppor commented Jan 27, 2025

Linking a pull request to an issue using a keyword

Example:

koppor commented Jan 27, 2025

koppor commented Jan 27, 2025

koppor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

koppor commented Jan 27, 2025

nawalchahboune commented Feb 11, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 12, 2025

trag-bot bot Mar 12, 2025

Choose a reason for hiding this comment

trag-bot bot Mar 12, 2025

Choose a reason for hiding this comment

koppor commented Mar 12, 2025

calixtus commented Mar 13, 2025

khola22 commented Dec 2, 2024 •

edited by koppor

Loading

subhramit commented Jan 13, 2025 •

edited

Loading