Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add (simple) git support #578

Open
koppor opened this issue Jun 14, 2022 · 28 comments
Open

Add (simple) git support #578

koppor opened this issue Jun 14, 2022 · 28 comments

Comments

@koppor
Copy link
Owner

koppor commented Jun 14, 2022

As user, I would like to have a seamless integration in git. For "straight-forward" cases, JabRef should do "magic".

Implementation drivers

  • In case of an error, the file should be in the state before JabRef started to do some wizard thing

Opening a library

  1. When opening a library (.bib file), JabRef checks whether the .bib file is contained in a git repository. If not in a git repository, no further action is done. Otherwise it is tagged as "versioned" (BibDatabaseContext)
  2. JabRef exetues a "git pull" to update the .bib file
  3. If error: notification of error, revert file to previous state
  4. JabRef opens the updated file

Saving of a library

No action taken (to save number of commits)

Saving a library

  1. JabRef saves the .bib file
  2. A git commit is created. git commit message "Automatic update via JabRef".
  3. git pull --rebase is executed.
  4. In case of an error, revert to commit created at step 2 and try git pull
  5. In case of an error, revert to commit created at step 2 and ouput a notification
  6. Then, git push
  7. In case of an error, output error
  8. Output "Successfully pushed"

Advanced implementation

A user might want to trigger pushing

  • Reuse button "synchronize" of shared database. This button has the same functionality as "Saving a library"
  • Let user configure frequency of push. At this frequence, on "Saving a library", a commit and the steps of "Saving a library" are done.
@koppor
Copy link
Owner Author

koppor commented May 21, 2023

At JabRef#8431 a user described his issues with current JabRef not having any git support.

@lbenicio
Copy link

lbenicio commented Sep 1, 2023

Hi, my colleague and I are students from the University of São Paulo in Brazil, and we were asked to help in the open-source community. We would like to take a look into this issue and try to implement it. Could we have it assigned to me? Do you have any information to share that would be nice to know beforehand?

@koppor
Copy link
Owner Author

koppor commented Sep 1, 2023

Dear Leonardo! Thank you for choosing JabRef! I reserved the issue for you.

Please note that this is a big project.

grafik

I shared information for behavior in the issue description. You can start investigating current behavior at SaveDatabaseAction.java.

@koppor
Copy link
Owner Author

koppor commented Sep 1, 2023

Here our general newcomer text (which is also information to share)

As a general advice for newcomers: check out Contributing for a start. Also, guidelines for setting up a local workspace is worth having a look at.

Feel free to ask here at GitHub, if you have any issue related questions. If you have questions about how to setup your workspace use JabRef's Gitter chat. Try to open a (draft) pull-request early on, so that people can see you are working on the issue and so that they can see the direction the pull request is heading towards. This way, you will likely receive valuable feedback.

@koppor
Copy link
Owner Author

koppor commented Sep 7, 2023

@lbenicio Any updates? Any UML diagrams? Concepts? UI sketches? Initial code?

@lbenicio
Copy link

Hi, sorry for the lack of updates.

From now, the idea for the simple implementation seems simple. Here what we have:

  1. Verify .git
  2. call git sync method
  3. issue the commands
  4. on failure, open a UI.

We were struggling with the point of external user interaction with the git to understand if it changes any behavior.

Now we are looking into how to open the UI with the changes.

From what I understand, we should present a solution proposal and then follow up with the code, right?

We were scheming the solution, but we did not have a diagram. We are on 2 people working 4h/week/each on the project, so the progress may be a little slow?

But what concerns me is that we did not figure what the complexity of the project yet. It looks like direct implementation.

Should we “reserve” an issue just when we get a proper solution?

@koppor
Copy link
Owner Author

koppor commented Sep 12, 2023

Hi, sorry for the lack of updates.

No worries.

From now, the idea for the simple implementation seems simple. Here what we have:

1. Verify .git
2. call git sync method
3. issue the commands
4. on failure, open a UI.

Sounds OK. No need for a real UI. We have "dialogService". Example:

    dialogService.notify(Localization.lang("Issue on GitHub successfully reported."));

We were struggling with the point of external user interaction with the git to understand if it changes any behavior.

I don't know, which of these steps outlined above are unclear:

  1. JabRef saves the .bib file
  2. A git commit is created. git commit message "Automatic update via JabRef".
  3. git pull --rebase is executed.
  4. In case of an error, revert to commit created at step 2 and try git pull
  5. In case of an error, revert to commit created at step 2 and ouput a notification
  6. Then, git push
  7. In case of an error, output error
  8. Output "Successfully pushed"

From what I understand, we should present a solution proposal and then follow up with the code, right?

In Software Engineering, one uses diagrams etc to communicate. UML sequence diagrams are a good thing to learn. -- You can also write code and submit a work-in-progress pull request to JabRef. To discuss things.

We were scheming the solution, but we did not have a diagram. We are on 2 people working 4h/week/each on the project, so the progress may be a little slow?

I don't know about your final date. Slow progress is very OK. Better 4h "deep work" hours than 8 non-focussed ones. I would suggest that you come back with intermediate results after each working block. We can open a separate skype group for you if you want.

I would also like to share the thought that this is a work educating you to become a better programmer and a better software architect. One needs 10.000 hours to become an expert. Thus, each hour invested here brings you closer to the goal becoming an expert.

In October, there is #hacktoberfest. I would recommend to use that opportunity to level-up your coding skills even more.

But what concerns me is that we did not figure what the complexity of the project yet. It looks like direct implementation.

The project is flagged as "large", because it is not straight-forward code. One needs to understand the current saving code and where to plug-in. It could be that the final "productive" code is 50 lines of code. Nevertheless, there will be 500 to 1000 lines of code for testing (JUnit tests, ...).

Should we “reserve” an issue just when we get a proper solution?

It is very OK to reserve an issue. We take back the reservation if the solution is implemented :) or the student resigns. We have enough other issues where other persons can work on.

@lbenicio
Copy link

Great, thank you the help and very detailed attention! Much appreciated!

In Software Engineering, one uses diagrams etc to communicate. UML sequence diagrams are a good thing to learn. -- You can also write code and submit a work-in-progress pull request to JabRef. To discuss things.

We will try to submit a work in progress solution/diagram to exemplify the solution as fast as possible.

The project is flagged as "large", because it is not straight-forward code. One needs to understand the current saving code and where to plug-in. It could be that the final "productive" code is 50 lines of code. Nevertheless, there will be 500 to 1000 lines of code for testing (JUnit tests, ...).

perfect, I believe we are on the right track.

I will take the opportunity to introduce my partner @marcelojunior1

@marcelojunior1
Copy link

Hi @koppor , I'm Marcelo and Leonardo's partner in our class. Thank you for your support and we hope to be able to contribute to JabRef.

@koppor
Copy link
Owner Author

koppor commented Sep 14, 2023

@marcelojunior1 also welcome on bord!

I think, the place in the code is /src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java#L257

The are already test cases for SaveDatabaseAction. You can add git-based tests.

For another project, I needed to create a git repository within JUnit tests. Maybe, the code is an inspiration for your tests, too? -- https://github.com/koppor/heylogs/blob/34c84ebb101b310bd0d715c669ab38e6d0300e65/heylogs-api/src/test/java/internal/heylogs/GitDiffRuleTest.java#L120

@lbenicio
Copy link

Great! The help is much appreciated, thank you!

We intended to update the git handler class and use it to manage this kind of git repositories as well.

About our progress: we are just finishing our solution proposal

Yes, the tests are really the complexity here, and this will help a lot.

@lbenicio
Copy link

After a long wait, here is our solution proposal:

fluxograma-Page-1 drawio-2

We should start writing unit tests next week.

Maybe we add our code to an “intermediate” pull request, so you can track our progress

@koppor
Copy link
Owner Author

koppor commented Sep 18, 2023

Thank you for sharing the UML flow chart.

I have following comments:

  • Name the .bib file "bib file" - "bibbi" seems something non-English.
  • I don't see how "commit bibbi file" can work without any saving (I think, the file should be saved before any interaction with a git remote. Thus, "saveDatabase()" should be called, and then the git "magic" handled
  • My proposal with "git rebase" was an advanced thing. Maybe drop that completely and just to a "git pull" with merge strategy.
  • In case there is merge conflict at pulling, reset to the local version - and just output a warning error that the user needs to resolve the conflicts for themselves.

Code hint: On org.jabref.logic.git.GitHandler you see some git magic. I think, that class cannot be used directly: It creates a git repository on start. Either you modify it - or your start with your own class. Your own class could limit the the life cycle of the GitRepository when doing a save - and then throw away that handle (try-with-resources).

Notes

  1. you need to suspend the file change listener. Otherwise, JabRef will start nagging that the file changed on disk (during your git operations)
  2. you need to make JabRef aware of the remote changes: The file needs to be loaded again in JabRef. Search for the usage of org.jabref.gui.collab.DatabaseChangesResolverDialog.

Advanced: The SLR functionality of JabRef (included in org.jabref.logic.git.GitHandler) has support to handle patches (SlrGitHandler.java#L95). I think, this cannot be directly used, but I have not thought about it.

The core idea should be to find the modified entries by the citation key (AKA BibTeX key) locally and remotly. And then displaying a merge entries dialog if the bibtex key is remote and locally the same, but different content. -- You can call the org.jabref.gui.collab.DatabaseChangesResolverDialog for that.

One could even reuse that dialog:

Put in a flow:

  • save the library
  • pull changes
  • conflicts exist: Revert changes
  • start file-changed-on-disk listener
  • put state of remote on disk
  • JabRef will trigger the DatabaseChangesResolverDialog

@koppor
Copy link
Owner Author

koppor commented Sep 18, 2023

Screenshot of the Merge Entries dialog (as of today)

image

@lbenicio
Copy link

Wow, great, thanks!

I don't see how "commit bibbi file" can work without any saving (I think, the file should be saved before any interaction with a git remote. Thus, "saveDatabase()" should be called, and then the git "magic" handled

Totally forgot that. You are correct. We should wait for file save before calling for git save.

you need to make JabRef aware of the remote changes: The file needs to be loaded again in JabRef. Search for the usage of org.jabref.gui.collab.DatabaseChangesResolverDialog.

Forgot that too. Totally makes sense to update JabRef internal database with the updated changes from git pull.

I will update our flow chart to reflect the changes and post here.

@koppor
Copy link
Owner Author

koppor commented Sep 19, 2023

Thank you! - As a side comment: Please ensure that gitk is running on your machine. Then you can view the state of the repository easily. Tutorial at: https://lostechies.com/joshuaflanagan/2010/09/03/use-gitk-to-understand-git/.

@lbenicio
Copy link

fluxograma-Page-1 drawio-3

So, this is the final idea. The user should be on a loop to fix conflicts until the user cancels the action, and we summarize to the normal flow and finish the git save method.

From this, we believe we should have an implementation and test strategy (which would be bigger then the code itself)

@marcelojunior1 marcelojunior1 removed their assignment Sep 20, 2023
@marcelojunior1
Copy link

Sorry, I accidentally remove my assignment. :(

@koppor
Copy link
Owner Author

koppor commented Sep 20, 2023

Regarding the diagram. I thought, it was an UML flow chart, now you are using another syntax. We computer scientists are picky on the syntax of things. Please choose an existing thing (and do not re-invent something else). Example: Casing OUTPUT and output. I think, the extra casing is not necessary. -- Round circles versus boxes versus boxes with rounded corners versus a yellow box. -- Stick with plain rectangles please.

I put the comments straight, I hope, this is OK

  • Do not put method names inside the flow chart. On that level, you are not sure which method your code will be in. Moreover, saveGitDatabase() is a method, you will implement. The flowchart suggests that some git checking happens afterwards. This is strange. Just remove saveGitDatabase()
  • Flow charts do not have the parallel semantics. Thus, either change the diagram syntax or think whether there is something wrong. I would just continue at "revert to the local version". The dialog itself tells the user that something was wrong. Two different things happening confuse users
  • There is no "stop file-changed-ondisk listener" in the diagram.
  • No arrow from "JabREf will trigger ..." to "merge conflicts". The flow ends here
  • No arrow from "output successful message" to "save database". Because: The flow is started by the user triggering the save. It is NOT an endless loop.

@lbenicio
Copy link

Ok, great! Yes, we decided not to use a proper diagram format, it was supposed to a sketch sorry, it was not a good idea.

Screenshot 2023-09-21 at 22 43 41

We got some progress, and managed to create the commit, we should submit a “preview” PR, so you can track our progress soon.

@koppor
Copy link
Owner Author

koppor commented Sep 25, 2023

Ok, great! Yes, we decided not to use a proper diagram format, it was supposed to a sketch sorry, it was not a good idea.

10.000 hours to become an expert. Drawing one diagram and getting feedback makes you a better diagrammer. Diagrams are useful for software developers and architects. Keep practicing!

Note that drawing and discussing a diagram takes shorter time than implementing everything. This is the main benefit of diagrams.

We got some progress, and managed to create the commit, we should submit a “preview” PR, so you can track our progress soon.

Nice. Looking forward!

@marcelojunior1
Copy link

marcelojunior1 commented Sep 26, 2023

Note that drawing and discussing a diagram takes shorter time than implementing everything. This is the main benefit of diagrams.

Yes, we agree! We were trying to do push/commit operations using ssh, but it was very complicated to import and configure org.eclipse.jgit.ssh.apache package. Now it's working and we can think more about the diagram.

@lbenicio
Copy link

We just submitted a pull request, so everybody can track our progress and provide suggestions.

JabRef#10422

@lbenicio
Copy link

lbenicio commented Oct 8, 2023

Hi @koppor

Writing to update you about our progress. We almost finished the implementation and are moving to tests. We got the preferences' menu working and the git ssh, and user/pass login working as well as the login retrieval from env variables and preferences menu.

Next sprint, we are moving to unit tests (if nothing unexpected appears)

Our ETA would be 4 weeks to finish everything. I hope we can make it (since our semester would end).

@koppor
Copy link
Owner Author

koppor commented Oct 13, 2023

Thank you for the update. It would be nice if you reopened the PR or commented if the time budget is spend on your side. No need to implement GPG signing. Just get the "basic" functionality running properly. Since this touches a core of JabRef, clean and working code is more important than features.

Especially, the user experience has to be great ^^. One can check how other tools do it. At https://www.reddit.com/r/LaTeX/comments/1179mdx/texstudio_git_integration_for_easy_committing/ there is a good explanation for TeXStuido.

Side note: I would have relied on https only, but OK for me that you worked on SSH, too. 😅

@lbenicio
Copy link

Ok, finally, we finished all unit testing, e2e testing and submited the PR.
JabRef#10586

@lbenicio
Copy link

@koppor just to let you know, we did not abandon the project. We are in the final weeks, many finals tests to do, we missed the class dates to finish the PR, but we are going to finish it. Maybe the first or second week of December.

@koppor
Copy link
Owner Author

koppor commented Apr 16, 2024

For requirements refinement, please read on related work, e.g., TeXStudio git integration: https://www.reddit.com/j9az1u9?utm_source=share&utm_medium=android_app&utm_name=androidcss&utm_term=1&utm_content=2

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 a pull request may close this issue.

4 participants