Re: [PATCH 2/3] Move copy_note_for_rewrite + friends from builtin/notes.c to notes-utils.c
On Thu, Jun 13, 2013 at 1:45 AM, Andreas Krey wrote: > On Wed, 12 Jun 2013 13:28:05 +, Felipe Contreras wrote: > ... >> And you are >> doing that with the express purpose of annoying. > > Where did 'assume good faith' go to today? Did you read the last part? "This does not mean that one should continue to assume good faith when there's evidence to the contrary." That being said, my evidence was not solid, and while there is still the possibility that he was indeed acting in good faith, I've received no response from him, and Junio has committed the change without any mentioning of where the idea come from. Either way, I bet you my good faith suggestion will *not* end up in the official guidelines, nor will any suggestion of mine. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] Move copy_note_for_rewrite + friends from builtin/notes.c to notes-utils.c
On Wed, 12 Jun 2013 13:28:05 +, Felipe Contreras wrote: ... > And you are > doing that with the express purpose of annoying. Where did 'assume good faith' go to today? Andreas -- "Totally trivial. Famous last words." From: Linus Torvalds Date: Fri, 22 Jan 2010 07:29:21 -0800 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] Move copy_note_for_rewrite + friends from builtin/notes.c to notes-utils.c
Johan Herland writes: > This is a pure code movement of the machinery for copying notes to > rewritten objects. This code was located in builtin/notes.c for > historical reasons. In order to make it available to builtin/commit.c > it was declared in builtin.h. This was more of an accident of history > than a concious design, and we now want to make this machinery more > widely available. > > Hence, this patch moves the code into the new notes-utils.[hc] files > which are included into libgit.a. Except for adjusting #includes > accordingly, this patch merely moves the relevant functions verbatim > into the new files. > > Cc: Thomas Rast > Signed-off-by: Johan Herland > --- > Makefile | 2 + > builtin.h| 16 --- > builtin/commit.c | 1 + > builtin/notes.c | 131 +- > notes-utils.c| 132 > +++ > notes-utils.h| 23 ++ > 6 files changed, 159 insertions(+), 146 deletions(-) > create mode 100644 notes-utils.c > create mode 100644 notes-utils.h Output from "git show -C1 --stat" after applying this patch shows mostly removals (i.e. builtin/notes.c loses what was lifted from it, notes-utils.c starts its life as a copy of the former and the patch shows removal of what should not move to notes-utils.c). After inspecting "added" lines to these two files, I did not spot anything suspicious, except for one C++/C99 comment (will locally touch-up). Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] Move copy_note_for_rewrite + friends from builtin/notes.c to notes-utils.c
Johan Herland wrote: > On Wed, Jun 12, 2013 at 8:28 PM, Felipe Contreras > wrote: > > On Wed, Jun 12, 2013 at 2:10 AM, Johan Herland wrote: > >> On Wed, Jun 12, 2013 at 2:32 AM, Felipe Contreras > >> wrote: > >>> On Tue, Jun 11, 2013 at 7:13 PM, Johan Herland wrote: > This is a pure code movement of the machinery for copying notes to > rewritten objects. This code was located in builtin/notes.c for > historical reasons. In order to make it available to builtin/commit.c > it was declared in builtin.h. This was more of an accident of history > than a concious design, and we now want to make this machinery more > widely available. > > Hence, this patch moves the code into the new notes-utils.[hc] files > which are included into libgit.a. Except for adjusting #includes > accordingly, this patch merely moves the relevant functions verbatim > into the new files. > > Cc: Thomas Rast > Signed-off-by: Johan Herland > >>> > >>> I wonder where you got that idea from. Did you come up with that out thin > >>> air? > >> > >> Obviously not. I should add > >> > >> Suggested-by: Junio C Hamano > > > > You are still not explaining where the idea came from. And you are > > doing that with the express purpose of annoying. > > Truly, I am not trying to annoy anyone. I have not followed the > preceding discussion closely, and I wrote the patch based solely on > one paragraph from Junio's email[1]. Here is another pagraph: > Moving sequencer.c to builtin/ is not even a solution. Linking > git-upload-pack will still pull in builtin/notes.o along with cmd_notes(), > which is not called from main(); as you remember, cmd_foo() in all > builtin/*.o are designed to be called from git.c::main(). Which clearly refers to: http://article.gmane.org/gmane.comp.version-control.git/226752 > > Where did the idea come from? > > I got it from Junio. I do not know if I might have accidentally > plagiarized something you already submitted to the mailing list, > although I would be surprised if that was the case, since - as far as > I understand - you are opposed to this solution. You are aware I opposed this *solution*, yet were not aware that I sent the first patch in this thread, which clearly states the *problem*? > This way there will not be linking issues when top-level objects try to > access functions of builtin objects. http://article.gmane.org/gmane.comp.version-control.git/226845 > Originally-envisioned-by: Felipe Contreras ? Do I have to do it for you? Your commit message is all wrong, because nowhere are you pointing out *why* you are making the change. --- Move copy_note_for_rewrite + friends to notes-utils.c In order to make these functionas available to top-level objects (e.g. sequencer.o), we need to move them out of the builtin/ subdirectory. Reported-by: Felipe Contreras --- -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] Move copy_note_for_rewrite + friends from builtin/notes.c to notes-utils.c
On Wed, Jun 12, 2013 at 8:28 PM, Felipe Contreras wrote: > On Wed, Jun 12, 2013 at 2:10 AM, Johan Herland wrote: >> On Wed, Jun 12, 2013 at 2:32 AM, Felipe Contreras >> wrote: >>> On Tue, Jun 11, 2013 at 7:13 PM, Johan Herland wrote: This is a pure code movement of the machinery for copying notes to rewritten objects. This code was located in builtin/notes.c for historical reasons. In order to make it available to builtin/commit.c it was declared in builtin.h. This was more of an accident of history than a concious design, and we now want to make this machinery more widely available. Hence, this patch moves the code into the new notes-utils.[hc] files which are included into libgit.a. Except for adjusting #includes accordingly, this patch merely moves the relevant functions verbatim into the new files. Cc: Thomas Rast Signed-off-by: Johan Herland >>> >>> I wonder where you got that idea from. Did you come up with that out thin >>> air? >> >> Obviously not. I should add >> >> Suggested-by: Junio C Hamano > > You are still not explaining where the idea came from. And you are > doing that with the express purpose of annoying. Truly, I am not trying to annoy anyone. I have not followed the preceding discussion closely, and I wrote the patch based solely on one paragraph from Junio's email[1]. > Where did the idea come from? I got it from Junio. I do not know if I might have accidentally plagiarized something you already submitted to the mailing list, although I would be surprised if that was the case, since - as far as I understand - you are opposed to this solution. Furthermore, I thought you might not like having your name mentioned in a patch you do not agree with, but if you think differently I have no problem adding your name. I don't know what kind of attribution you would prefer though: Originally-envisioned-by: Felipe Contreras ? NAKed-by: Felipe Contreras ? Something else? ...Johan [1]: Quoted from <7vehc8a05n@alter.siamese.dyndns.org>: On Tue, Jun 11, 2013 at 9:59 PM, Junio C Hamano wrote: > Linus Torvalds writes: > 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. -- Johan Herland, www.herland.net -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] Move copy_note_for_rewrite + friends from builtin/notes.c to notes-utils.c
On Wed, Jun 12, 2013 at 2:10 AM, Johan Herland wrote: > On Wed, Jun 12, 2013 at 2:32 AM, Felipe Contreras > wrote: >> On Tue, Jun 11, 2013 at 7:13 PM, Johan Herland wrote: >>> This is a pure code movement of the machinery for copying notes to >>> rewritten objects. This code was located in builtin/notes.c for >>> historical reasons. In order to make it available to builtin/commit.c >>> it was declared in builtin.h. This was more of an accident of history >>> than a concious design, and we now want to make this machinery more >>> widely available. >>> >>> Hence, this patch moves the code into the new notes-utils.[hc] files >>> which are included into libgit.a. Except for adjusting #includes >>> accordingly, this patch merely moves the relevant functions verbatim >>> into the new files. >>> >>> Cc: Thomas Rast >>> Signed-off-by: Johan Herland >> >> I wonder where you got that idea from. Did you come up with that out thin >> air? > > Obviously not. I should add > > Suggested-by: Junio C Hamano You are still not explaining where the idea came from. And you are doing that with the express purpose of annoying. Where did the idea come from? -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] Move copy_note_for_rewrite + friends from builtin/notes.c to notes-utils.c
On Wed, Jun 12, 2013 at 2:32 AM, Felipe Contreras wrote: > On Tue, Jun 11, 2013 at 7:13 PM, Johan Herland wrote: >> This is a pure code movement of the machinery for copying notes to >> rewritten objects. This code was located in builtin/notes.c for >> historical reasons. In order to make it available to builtin/commit.c >> it was declared in builtin.h. This was more of an accident of history >> than a concious design, and we now want to make this machinery more >> widely available. >> >> Hence, this patch moves the code into the new notes-utils.[hc] files >> which are included into libgit.a. Except for adjusting #includes >> accordingly, this patch merely moves the relevant functions verbatim >> into the new files. >> >> Cc: Thomas Rast >> Signed-off-by: Johan Herland > > I wonder where you got that idea from. Did you come up with that out thin air? Obviously not. I should add Suggested-by: Junio C Hamano ...Johan -- Johan Herland, www.herland.net -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] Move copy_note_for_rewrite + friends from builtin/notes.c to notes-utils.c
On Tue, Jun 11, 2013 at 7:13 PM, Johan Herland wrote: > This is a pure code movement of the machinery for copying notes to > rewritten objects. This code was located in builtin/notes.c for > historical reasons. In order to make it available to builtin/commit.c > it was declared in builtin.h. This was more of an accident of history > than a concious design, and we now want to make this machinery more > widely available. > > Hence, this patch moves the code into the new notes-utils.[hc] files > which are included into libgit.a. Except for adjusting #includes > accordingly, this patch merely moves the relevant functions verbatim > into the new files. > > Cc: Thomas Rast > Signed-off-by: Johan Herland I wonder where you got that idea from. Did you come up with that out thin air? And here goes my bet; nobody will ever use these notes-utils outside of the git binary. Ever. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] Move copy_note_for_rewrite + friends from builtin/notes.c to notes-utils.c
This is a pure code movement of the machinery for copying notes to
rewritten objects. This code was located in builtin/notes.c for
historical reasons. In order to make it available to builtin/commit.c
it was declared in builtin.h. This was more of an accident of history
than a concious design, and we now want to make this machinery more
widely available.
Hence, this patch moves the code into the new notes-utils.[hc] files
which are included into libgit.a. Except for adjusting #includes
accordingly, this patch merely moves the relevant functions verbatim
into the new files.
Cc: Thomas Rast
Signed-off-by: Johan Herland
---
Makefile | 2 +
builtin.h| 16 ---
builtin/commit.c | 1 +
builtin/notes.c | 131 +-
notes-utils.c| 132 +++
notes-utils.h| 23 ++
6 files changed, 159 insertions(+), 146 deletions(-)
create mode 100644 notes-utils.c
create mode 100644 notes-utils.h
diff --git a/Makefile b/Makefile
index 0f931a2..22deee1 100644
--- a/Makefile
+++ b/Makefile
@@ -682,6 +682,7 @@ LIB_H += merge-recursive.h
LIB_H += mergesort.h
LIB_H += notes-cache.h
LIB_H += notes-merge.h
+LIB_H += notes-utils.h
LIB_H += notes.h
LIB_H += object.h
LIB_H += pack-refs.h
@@ -815,6 +816,7 @@ LIB_OBJS += name-hash.o
LIB_OBJS += notes.o
LIB_OBJS += notes-cache.o
LIB_OBJS += notes-merge.o
+LIB_OBJS += notes-utils.o
LIB_OBJS += object.o
LIB_OBJS += pack-check.o
LIB_OBJS += pack-refs.o
diff --git a/builtin.h b/builtin.h
index 78fb14d..72bb2a8 100644
--- a/builtin.h
+++ b/builtin.h
@@ -5,7 +5,6 @@
#include "strbuf.h"
#include "cache.h"
#include "commit.h"
-#include "notes.h"
#define DEFAULT_MERGE_LOG_LEN 20
@@ -23,21 +22,6 @@ struct fmt_merge_msg_opts {
extern int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
struct fmt_merge_msg_opts *);
-struct notes_rewrite_cfg {
- struct notes_tree **trees;
- const char *cmd;
- int enabled;
- combine_notes_fn combine;
- struct string_list *refs;
- int refs_from_env;
- int mode_from_env;
-};
-
-struct notes_rewrite_cfg *init_copy_notes_for_rewrite(const char *cmd);
-int copy_note_for_rewrite(struct notes_rewrite_cfg *c,
- const unsigned char *from_obj, const unsigned char
*to_obj);
-void finish_copy_notes_for_rewrite(struct notes_rewrite_cfg *c, const char
*msg);
-
extern int textconv_object(const char *path, unsigned mode, const unsigned
char *sha1, int sha1_valid, char **buf, unsigned long *buf_size);
extern int cmd_add(int argc, const char **argv, const char *prefix);
diff --git a/builtin/commit.c b/builtin/commit.c
index f8df8ca..ce40176 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -29,6 +29,7 @@
#include "gpg-interface.h"
#include "column.h"
#include "sequencer.h"
+#include "notes-utils.h"
static const char * const builtin_commit_usage[] = {
N_("git commit [options] [--] ..."),
diff --git a/builtin/notes.c b/builtin/notes.c
index 6a80714..9ed2508 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -18,9 +18,7 @@
#include "parse-options.h"
#include "string-list.h"
#include "notes-merge.h"
-
-static void commit_notes(struct notes_tree *t, const char *msg);
-static combine_notes_fn parse_combine_notes_fn(const char *v);
+#include "notes-utils.h"
static const char * const git_notes_usage[] = {
N_("git notes [--ref ] [list []]"),
@@ -287,133 +285,6 @@ static int parse_reedit_arg(const struct option *opt,
const char *arg, int unset
return parse_reuse_arg(opt, arg, unset);
}
-static void commit_notes(struct notes_tree *t, const char *msg)
-{
- struct strbuf buf = STRBUF_INIT;
- unsigned char commit_sha1[20];
-
- if (!t)
- t = &default_notes_tree;
- if (!t->initialized || !t->ref || !*t->ref)
- die(_("Cannot commit uninitialized/unreferenced notes tree"));
- if (!t->dirty)
- return; /* don't have to commit an unchanged tree */
-
- /* Prepare commit message and reflog message */
- strbuf_addstr(&buf, msg);
- if (buf.buf[buf.len - 1] != '\n')
- strbuf_addch(&buf, '\n'); /* Make sure msg ends with newline */
-
- create_notes_commit(t, NULL, &buf, commit_sha1);
- strbuf_insert(&buf, 0, "notes: ", 7); /* commit message starts at index
7 */
- update_ref(buf.buf, t->ref, commit_sha1, NULL, 0, DIE_ON_ERR);
-
- strbuf_release(&buf);
-}
-
-static combine_notes_fn parse_combine_notes_fn(const char *v)
-{
- if (!strcasecmp(v, "overwrite"))
- return combine_notes_overwrite;
- else if (!strcasecmp(v, "ignore"))
- return combine_notes_ignore;
- else if (!strcasecmp(v, "concatenate"))
- return combine_notes_concatenate;
- else if (!strcasecmp(v, "cat_sort_uniq"))
- return combine_note
