[PATCH] git object format: make sure parenthood isn't lost when saving
Linus Torvalds
torvalds at linux-foundation.org
Thu Mar 13 16:55:26 PDT 2014
From: Linus Torvalds <torvalds at linux-foundation.org>
Date: Thu, 13 Mar 2014 15:42:45 -0700
Subject: [PATCH] git object format: make sure parenthood isn't lost when saving
This makes subsurface remember the git source commit of the dive data.
If you save to an existing branch, subsurface will now complain and
refuse to save if you try to save if the existing branch is not related
to the original source. That would destroy the history of the dive
data, which in turn would make it impossible to do sane merging of the
data.
If you save to a new branch, it will see if the previous parent commit
is known in the repository you are saving to, and will save parenthood
information if so. Otherwise it will save it as a new parentless commit
("root commit" in git parlance).
Signed-off-by: Linus Torvalds <torvalds at linux-foundation.org>
---
This should do the rigth thing on saving git format history.
I *do* wonder if we should save the git parenthood ID into the XML file,
which could allow us to try to merge changes even past being saved into
XML (if they are then re-imported into the same repository later).
Also, while doing this, I noticed that we have absolutely horrible
behavior on save errors - we ignore them. This is not git-specific,
though. See "save_dives_logic()", which just throws all errors away.
It's particularly bad with the git save error handling, though, because
"report_error()" isn't actually connected to the GUI in any way (could
some GUI person perhaps do that?). And there are now more error conditions
than just "disk is full or read-only", since it can now say "hey, you're
trying to save to a branch with unrelated history".
It's pretty bad for regular XML file saving too, though.
dive.h | 4 ++++
load-git.c | 30 ++++++++++++++++++++++++-----
qt-ui/mainwindow.cpp | 1 +
save-git.c | 53 ++++++++++++++++++++++++++++++++++++++++------------
save-xml.c | 5 ++++-
5 files changed, 75 insertions(+), 18 deletions(-)
diff --git a/dive.h b/dive.h
index 5ac43f16dfb5..41b467d29b0b 100644
--- a/dive.h
+++ b/dive.h
@@ -686,10 +686,14 @@ extern void save_dives_logic(const char *filename, bool select_only);
extern void save_dive(FILE *f, struct dive *dive);
extern void export_dives_uddf(const char *filename, const bool selected);
+struct git_oid;
struct git_repository;
extern struct git_repository *is_git_repository(const char *filename, const char **branchp);
extern int git_save_dives(struct git_repository *, const char *, bool select_only);
extern int git_load_dives(struct git_repository *, const char *);
+extern const char *saved_git_id;
+extern void clear_git_id(void);
+extern void set_git_id(const struct git_oid *);
extern int subsurface_rename(const char *path, const char *newpath);
extern int subsurface_open(const char *path, int oflags, mode_t mode);
diff --git a/load-git.c b/load-git.c
index c268cf1ef093..12bf66141e88 100644
--- a/load-git.c
+++ b/load-git.c
@@ -14,6 +14,8 @@
#include "device.h"
#include "membuffer.h"
+const char *saved_git_id = NULL;
+
struct keyword_action {
const char *keyword;
void (*fn)(char *, struct membuffer *, void *);
@@ -1198,11 +1200,25 @@ static int load_dives_from_tree(git_repository *repo, git_tree *tree)
return 0;
}
+void clear_git_id(void)
+{
+ saved_git_id = NULL;
+}
+
+void set_git_id(const struct git_oid * id)
+{
+ static char git_id_buffer[GIT_OID_HEXSZ+1];
+
+ git_oid_tostr(git_id_buffer, sizeof(git_id_buffer), id);
+ saved_git_id = git_id_buffer;
+}
+
static int do_git_load(git_repository *repo, const char *branch)
{
int ret;
git_reference *ref;
- git_object *tree;
+ git_commit *commit;
+ git_tree *tree;
ret = git_branch_lookup(&ref, repo, branch, GIT_BRANCH_LOCAL);
if (ret) {
@@ -1210,10 +1226,14 @@ static int do_git_load(git_repository *repo, const char *branch)
if (ret)
return report_error("Unable to look up branch '%s'", branch);
}
- if (git_reference_peel(&tree, ref, GIT_OBJ_TREE))
- return report_error("Could not look up tree of branch '%s'", branch);
- ret = load_dives_from_tree(repo, (git_tree *) tree);
- git_object_free(tree);
+ if (git_reference_peel((git_object **)&commit, ref, GIT_OBJ_COMMIT))
+ return report_error("Could not look up commit of branch '%s'", branch);
+ if (git_commit_tree(&tree, commit))
+ return report_error("Could not look up tree of commit in branch '%s'", branch);
+ ret = load_dives_from_tree(repo, tree);
+ if (!ret)
+ set_git_id(git_commit_id(commit));
+ git_object_free((git_object *)tree);
return ret;
}
diff --git a/qt-ui/mainwindow.cpp b/qt-ui/mainwindow.cpp
index 4d4d0baa221f..b67de1588509 100644
--- a/qt-ui/mainwindow.cpp
+++ b/qt-ui/mainwindow.cpp
@@ -214,6 +214,7 @@ void MainWindow::on_actionClose_triggered()
ui.newProfile->setEmptyState();
/* free the dives and trips */
+ clear_git_id();
while (dive_table.nr)
delete_single_dive(0);
diff --git a/save-git.c b/save-git.c
index 4760fe32a6e0..ab00ba9519b3 100644
--- a/save-git.c
+++ b/save-git.c
@@ -741,6 +741,23 @@ static int create_git_tree(git_repository *repo, struct dir *root, bool select_o
}
/*
+ * See if we can find the parent ID that the git data came from
+ */
+static git_object *try_to_find_parent(const char *hex_id, git_repository *repo)
+{
+ git_oid object_id;
+ git_commit *commit;
+
+ if (!hex_id)
+ return NULL;
+ if (git_oid_fromstr(&object_id, hex_id))
+ return NULL;
+ if (git_commit_lookup(&commit, repo, &object_id))
+ return NULL;
+ return (git_object *)commit;
+}
+
+/*
* libgit2 revision 0.20 and earlier do not have the signature and
* message log arguments.
*/
@@ -769,15 +786,18 @@ static int create_new_commit(git_repository *repo, const char *branch, git_oid *
case GIT_EINVALIDSPEC:
return report_error("Invalid branch name '%s'", branch);
case GIT_ENOTFOUND: /* We'll happily create it */
- ref = NULL; parent = NULL;
+ ref = NULL;
+ parent = try_to_find_parent(saved_git_id, repo);
break;
case 0:
if (git_reference_peel(&parent, ref, GIT_OBJ_COMMIT))
return report_error("Unable to look up parent in branch '%s'", branch);
- /* If the parent commit has the same tree ID, do nothing */
- if (git_oid_equal(tree_id, git_commit_tree_id((const git_commit *) parent)))
- return 0;
+ if (saved_git_id) {
+ const git_oid *id = git_commit_id((const git_commit *) parent);
+ if (git_oid_strcmp(id, saved_git_id))
+ return report_error("The git branch does not match the git parent of the source");
+ }
/* all good */
break;
@@ -790,12 +810,21 @@ static int create_new_commit(git_repository *repo, const char *branch, git_oid *
if (git_signature_now(&author, "Subsurface", "subsurface at hohndel.org"))
return report_error("No user name configuration in git repo");
- put_format(&commit_msg, "Created by subsurface %s\n", VERSION_STRING);
- if (git_commit_create_v(&commit_id, repo, NULL, author, author, NULL, mb_cstring(&commit_msg), tree, parent != NULL, parent))
- return report_error("Git commit create failed (%s)", strerror(errno));
-
- if (git_commit_lookup(&commit, repo, &commit_id))
- return report_error("Could not look up newly created commit");
+ /* If the parent commit has the same tree ID, do not create a new commit */
+ if (parent && git_oid_equal(tree_id, git_commit_tree_id((const git_commit *) parent))) {
+ /* If the parent already came from the ref, the commit is already there */
+ if (ref)
+ return 0;
+ /* Else we do want to create the new branch, but with the old commit */
+ commit = (git_commit *) parent;
+ } else {
+ put_format(&commit_msg, "Created by subsurface %s\n", VERSION_STRING);
+ if (git_commit_create_v(&commit_id, repo, NULL, author, author, NULL, mb_cstring(&commit_msg), tree, parent != NULL, parent))
+ return report_error("Git commit create failed (%s)", strerror(errno));
+
+ if (git_commit_lookup(&commit, repo, &commit_id))
+ return report_error("Could not look up newly created commit");
+ }
if (!ref) {
if (git_branch_create(&ref, repo, branch, commit, 0, author, "Create branch"))
@@ -803,6 +832,7 @@ static int create_new_commit(git_repository *repo, const char *branch, git_oid *
}
if (git_reference_set_target(&ref, ref, &commit_id, author, "Subsurface save event"))
return report_error("Failed to update branch '%s'", branch);
+ set_git_id(&commit_id);
return 0;
}
@@ -850,8 +880,7 @@ static int do_git_save(git_repository *repo, const char *branch, bool select_onl
return report_error("git tree write failed");
/* And save the tree! */
- create_new_commit(repo, branch, &id);
- return 0;
+ return create_new_commit(repo, branch, &id);
}
/*
diff --git a/save-xml.c b/save-xml.c
index a27dcfa76357..ef7396bf9c31 100644
--- a/save-xml.c
+++ b/save-xml.c
@@ -601,8 +601,11 @@ void save_dives_logic(const char *filename, const bool select_only)
const char *branch;
git = is_git_repository(filename, &branch);
- if (git && !git_save_dives(git, branch, select_only))
+ if (git) {
+ /* error returns, anybody? */
+ git_save_dives(git, branch, select_only);
return;
+ }
try_to_backup(filename);
--
1.9.0
More information about the subsurface
mailing list