Thread (16 messages) 16 messages, 4 authors, 2016-06-15

Re: [PATCH 0/3] Refactor useful notes functions into notes-utils.[ch]

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:57:43
Subsystem: the rest · Maintainer: Linus Torvalds

Possibly related (same subject, not in this thread)

Johan Herland [off-list ref] writes:
quoted
There is only one right solution.  If a useful function is buried in
builtin/*.o as a historical accident (i.e. it started its life as a
helper for that particular command, and nobody else used it from
outside so far) and that makes it impossible to use the function
from outside builtin/*.o, refactor the function and its callers and
move it to libgit.a.
Here goes...

...Johan
With these three patches, if you apply the following skeleton patch
(lifted from $gmane/226851 and adjusted minimally to the change
these patches introduce), we can see that the link breakage Felipe
observed in the message:

    Felipe Contreras [off-list ref] writes in $gmane/226851:
    > What happens?
    > 
    > libgit.a(sequencer.o): In function `copy_notes':
    > /home/felipec/dev/git/sequencer.c:110: undefined reference to
    > `init_copy_notes_for_rewrite'
    > /home/felipec/dev/git/sequencer.c:114: undefined reference to
    > `finish_copy_notes_for_rewrite'

is gone.

    > It is not the first time, nor the last that top-level code needs
    > builtin code, and the solution is easy; organize the code.

And as I already said, the above is correct.  The problem and the
general approach to solve it correctly were identified in the
message.

But what followed was a nonsense, which ended up wastign everybody's
time:
... Alas, this
simple solution reject on the basis that we shouldn't organize the
code, because the code is not meant to be organized.
The proposed patch was rejected on the basis that it was organized
the code in a wrong way.  And your patch shows how it should be
done.

Thanks for doing it right.

-- skeleton patch --

 sequencer.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
diff --git a/sequencer.c b/sequencer.c
index ab6f8a7..4281466 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -14,6 +14,7 @@
 #include "merge-recursive.h"
 #include "refs.h"
 #include "argv-array.h"
+#include "notes-utils.h"
 
 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
 
@@ -979,6 +980,17 @@ static void save_opts(struct replay_opts *opts)
 	}
 }
 
+static void copy_notes(const char *name, const char *msg)
+{
+       struct notes_rewrite_cfg *cfg;
+
+       cfg = init_copy_notes_for_rewrite(name);
+       if (!cfg)
+               return;
+
+       finish_copy_notes_for_rewrite(cfg, msg);
+}
+
 static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
 {
 	struct commit_list *cur;
@@ -997,6 +1009,8 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
 			return res;
 	}
 
+	copy_notes("cherry-pick", "notes copied by cherry-pick");
+
 	/*
 	 * Sequence of picks finished successfully; cleanup by
 	 * removing the .git/sequencer directory
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help