Re: [PATCH v4 1/2] status: introduce status.short to enable --short by default
Junio C Hamano writes: > I'll queue this patch after tweaking the test part like this. I agree your version is better, thanks. Jorge: this means if you have to edit the patch further, you'll have to start with the version in Junio's pu. But hopefully you won't have to. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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/RFC] git-remote-mediawiki: new tool to preview local changes without pushing
Jeff King writes: > I was thinking that you would be self-contained inside the > contrib/mw-to-git directory, and therefore you would have to teach your > code how to install the Git module, and you could not longer just "cp > git-remote-mediawiki" into the right place to install it. > > But I think we have already crossed that bridge somewhat with Git.pm. > And if you add your module as perl/Git/MediaWiki.pm and use the existing > perl build system, then it is not any extra effort from the build > system. I'm not sure having perl/Git/MediaWiki.pm would be a good idea: this MediaWiki.pm would be really a mediawiki thing more than a Git thing, so the Git main tree probably want to stay away from it and keep it in contrib. But you should be able to use contrib/mw-to-git/perl/GitMediawiki.pm or something like that and chain to ../../perl/Makefile in contrib/mw-to-git/Makefile. Also, for now, git-remote-mediawiki works only after you run "make install" in Git's toplevel. I think that's ok, but it would be weird to be able to use/test git-remote-mediawiki only after doing a "make install" to deploy the new mediawiki Perl module. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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 v4 29/31] git-remote-mediawiki: Add a .perlcriticrc file
On Tue, Jun 11, 2013 at 6:18 PM, Célestin Matte wrote: > Such a file allows to configure perlcritic. > Here, it is used to prevent to remove many unwanted rules and configure one to s/to prevent// > remove unwanted warnings. > > Signed-off-by: Célestin Matte > Signed-off-by: Matthieu Moy > --- > contrib/mw-to-git/.perlcriticrc | 28 > 1 file changed, 28 insertions(+) > create mode 100644 contrib/mw-to-git/.perlcriticrc > > diff --git a/contrib/mw-to-git/.perlcriticrc b/contrib/mw-to-git/.perlcriticrc > new file mode 100644 > index 000..a1f8451 > --- /dev/null > +++ b/contrib/mw-to-git/.perlcriticrc > @@ -0,0 +1,28 @@ > +# These 3 rules demand to add the s, m and x flag to *every* regexp. This is > +# overkill and would be harmful for readability. > +[-RegularExpressions::RequireExtendedFormatting] > +[-RegularExpressions::RequireDotMatchAnything] > +[-RegularExpressions::RequireLineBoundaryMatching] > + > +# This rules says that builtin functions should not be called with > parenthesis s/rules/rule/ s/parenthesis/parentheses/ > +# e.g.: (taken from CPAN's documentation) > +# open($handle, '>', $filename); #not ok > +# open $handle, '>', $filename; #ok > +# Applying such a rule would mean modifying a huge number of lines for a > +# question of style. > +[-CodeLayout::ProhibitParensWithBuiltins] > + > +# This rule states that each system call should have its return value checked > +# The problem is that it includes the print call. Checking every print call's > +# return value would be harmful to the code readabilty. > +# This configuration keeps all default function but print. > +[InputOutput::RequireCheckedSyscalls] > +functions = open say close > + > +# This rules demands to add a dependancy for the Readonly module. This is not > +# wished. > +[-ValuesAndExpressions::ProhibitConstantPragma] > + > +# This rule is not really useful (rather a question of style) and produces > many > +# warnings among the code. > +[-ValuesAndExpressions::ProhibitNoisyQuotes] > -- -- 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 v4 09/31] git-remote-mediawiki: Change the behaviour of a split
On Tue, Jun 11, 2013 at 6:18 PM, Célestin Matte
wrote:
> A "split ' '" is turned into a "split / /", which changes its behaviour: the
> old method matched a run of whitespaces (/\s*/), while the new one will match
> a
> single whitespace, which is what we want here. Indeed, in other contexts,
I missed this nit in the last round. '/ /' will match a exactly one
space (not an arbitrary whitespace character), so this really should
be: s/single whitespace/single space/
> changing split(' ') to split(/ /) could potentially be a regression, however,
> here, when parsing the output of "rev-list --parents", whose output SHA-1's
> are
> each separated by a single space, splitting on a single space is perfectly
> correct.
>
> Signed-off-by: Célestin Matte
> Signed-off-by: Matthieu Moy
--
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
[PATCH 1/3] finish_copy_notes_for_rewrite(): Let caller provide commit message
When copying notes for a rewritten object, the resulting notes commit
would have the following hardcoded commit message:
Notes added by 'git notes copy'
This is obviously bogus when the notes rewriting is performed by
'git commit --amend'.
Therefore, let the caller specify an appropriate notes commit message
instead of hardcoding it. The above message is used for 'git notes copy',
but when calling finish_copy_notes_for_rewrite() from builtin/commit.c,
we use the following message instead:
Notes added by 'git commit --amend'
Cc: Thomas Rast
Signed-off-by: Johan Herland
---
builtin.h| 2 +-
builtin/commit.c | 2 +-
builtin/notes.c | 9 +
3 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/builtin.h b/builtin.h
index faef559..78fb14d 100644
--- a/builtin.h
+++ b/builtin.h
@@ -36,7 +36,7 @@ struct notes_rewrite_cfg {
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);
+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);
diff --git a/builtin/commit.c b/builtin/commit.c
index d2f30d9..f8df8ca 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1591,7 +1591,7 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
if (cfg) {
/* we are amending, so current_head is not NULL */
copy_note_for_rewrite(cfg, current_head->object.sha1,
sha1);
- finish_copy_notes_for_rewrite(cfg);
+ finish_copy_notes_for_rewrite(cfg, "Notes added by 'git
commit --amend'");
}
run_rewrite_hook(current_head->object.sha1, sha1);
}
diff --git a/builtin/notes.c b/builtin/notes.c
index 57748a6..6a80714 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -403,11 +403,11 @@ int copy_note_for_rewrite(struct notes_rewrite_cfg *c,
return ret;
}
-void finish_copy_notes_for_rewrite(struct notes_rewrite_cfg *c)
+void finish_copy_notes_for_rewrite(struct notes_rewrite_cfg *c, const char
*msg)
{
int i;
for (i = 0; c->trees[i]; i++) {
- commit_notes(c->trees[i], "Notes added by 'git notes copy'");
+ commit_notes(c->trees[i], msg);
free_notes(c->trees[i]);
}
free(c->trees);
@@ -420,6 +420,7 @@ static int notes_copy_from_stdin(int force, const char
*rewrite_cmd)
struct notes_rewrite_cfg *c = NULL;
struct notes_tree *t = NULL;
int ret = 0;
+ const char *msg = "Notes added by 'git notes copy'";
if (rewrite_cmd) {
c = init_copy_notes_for_rewrite(rewrite_cmd);
@@ -461,10 +462,10 @@ static int notes_copy_from_stdin(int force, const char
*rewrite_cmd)
}
if (!rewrite_cmd) {
- commit_notes(t, "Notes added by 'git notes copy'");
+ commit_notes(t, msg);
free_notes(t);
} else {
- finish_copy_notes_for_rewrite(c);
+ finish_copy_notes_for_rewrite(c, msg);
}
return ret;
}
--
1.8.1.3.704.g33f7d4f
--
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 0/3] Refactor useful notes functions into notes-utils.[ch]
> 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 Johan Herland (3): finish_copy_notes_for_rewrite(): Let caller provide commit message Move copy_note_for_rewrite + friends from builtin/notes.c to notes-utils.c Move create_notes_commit() from notes-merge.c into notes-utils.c Makefile | 2 + builtin.h| 16 -- builtin/commit.c | 3 +- builtin/notes.c | 136 ++- notes-merge.c| 27 +- notes-merge.h| 14 - notes-utils.c| 157 +++ notes-utils.h| 37 + 8 files changed, 203 insertions(+), 189 deletions(-) create mode 100644 notes-utils.c create mode 100644 notes-utils.h -- 1.8.1.3.704.g33f7d4f -- 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 3/3] Move create_notes_commit() from notes-merge.c into notes-utils.c
create_notes_commit() is needed by both the notes-merge code, and by
commit_notes() in notes-utils. Since it is generally useful, and not
bound to the notes-merge machinery, we move it from (the more specific)
notes-merge to (the more general) notes-utils.
Signed-off-by: Johan Herland
---
notes-merge.c | 27 +--
notes-merge.h | 14 --
notes-utils.c | 27 ++-
notes-utils.h | 14 ++
4 files changed, 41 insertions(+), 41 deletions(-)
diff --git a/notes-merge.c b/notes-merge.c
index 0f67bd3..ab18857 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -9,6 +9,7 @@
#include "notes.h"
#include "notes-merge.h"
#include "strbuf.h"
+#include "notes-utils.h"
struct notes_merge_pair {
unsigned char obj[20], base[20], local[20], remote[20];
@@ -530,32 +531,6 @@ static int merge_from_diffs(struct notes_merge_options *o,
return conflicts ? -1 : 1;
}
-void create_notes_commit(struct notes_tree *t, struct commit_list *parents,
-const struct strbuf *msg, unsigned char *result_sha1)
-{
- unsigned char tree_sha1[20];
-
- assert(t->initialized);
-
- if (write_notes_tree(t, tree_sha1))
- die("Failed to write notes tree to database");
-
- if (!parents) {
- /* Deduce parent commit from t->ref */
- unsigned char parent_sha1[20];
- if (!read_ref(t->ref, parent_sha1)) {
- struct commit *parent = lookup_commit(parent_sha1);
- if (!parent || parse_commit(parent))
- die("Failed to find/parse commit %s", t->ref);
- commit_list_insert(parent, &parents);
- }
- /* else: t->ref points to nothing, assume root/orphan commit */
- }
-
- if (commit_tree(msg, tree_sha1, parents, result_sha1, NULL, NULL))
- die("Failed to commit notes tree to database");
-}
-
int notes_merge(struct notes_merge_options *o,
struct notes_tree *local_tree,
unsigned char *result_sha1)
diff --git a/notes-merge.h b/notes-merge.h
index 0c11b17..1d01f6a 100644
--- a/notes-merge.h
+++ b/notes-merge.h
@@ -26,20 +26,6 @@ struct notes_merge_options {
void init_notes_merge_options(struct notes_merge_options *o);
/*
- * Create new notes commit from the given notes tree
- *
- * Properties of the created commit:
- * - tree: the result of converting t to a tree object with write_notes_tree().
- * - parents: the given parents OR (if NULL) the commit referenced by t->ref.
- * - author/committer: the default determined by commmit_tree().
- * - commit message: msg
- *
- * The resulting commit SHA1 is stored in result_sha1.
- */
-void create_notes_commit(struct notes_tree *t, struct commit_list *parents,
-const struct strbuf *msg, unsigned char *result_sha1);
-
-/*
* Merge notes from o->remote_ref into o->local_ref
*
* The given notes_tree 'local_tree' must be the notes_tree referenced by the
diff --git a/notes-utils.c b/notes-utils.c
index 2ae5cb2..9107c37 100644
--- a/notes-utils.c
+++ b/notes-utils.c
@@ -2,7 +2,32 @@
#include "commit.h"
#include "refs.h"
#include "notes-utils.h"
-#include "notes-merge.h" // need create_notes_commit()
+
+void create_notes_commit(struct notes_tree *t, struct commit_list *parents,
+const struct strbuf *msg, unsigned char *result_sha1)
+{
+ unsigned char tree_sha1[20];
+
+ assert(t->initialized);
+
+ if (write_notes_tree(t, tree_sha1))
+ die("Failed to write notes tree to database");
+
+ if (!parents) {
+ /* Deduce parent commit from t->ref */
+ unsigned char parent_sha1[20];
+ if (!read_ref(t->ref, parent_sha1)) {
+ struct commit *parent = lookup_commit(parent_sha1);
+ if (!parent || parse_commit(parent))
+ die("Failed to find/parse commit %s", t->ref);
+ commit_list_insert(parent, &parents);
+ }
+ /* else: t->ref points to nothing, assume root/orphan commit */
+ }
+
+ if (commit_tree(msg, tree_sha1, parents, result_sha1, NULL, NULL))
+ die("Failed to commit notes tree to database");
+}
void commit_notes(struct notes_tree *t, const char *msg)
{
diff --git a/notes-utils.h b/notes-utils.h
index 0661e99..b4cb1bf 100644
--- a/notes-utils.h
+++ b/notes-utils.h
@@ -3,6 +3,20 @@
#include "notes.h"
+/*
+ * Create new notes commit from the given notes tree
+ *
+ * Properties of the created commit:
+ * - tree: the result of converting t to a tree object with write_notes_tree().
+ * - parents: the given parents OR (if NULL) the commit referenced by t->ref.
+ * - author/committer: the default determined by commmit_tree().
+ * - commit message: msg
+ *
+ * The resulting commit SHA1 is stored i
Re: [PATCH] Documentation/CommunityGuidelines
On Tue, Jun 11, 2013 at 3:46 PM, Philip Oakley wrote: > From: "Michael Haggerty" > Sent: Tuesday, June 11, 2013 7:52 PM > [...] > >> >> That's a very good point (and a good illustration, too). How do you >> like the new second and third sentences below? >> >> * When reviewing other peoples' code, be tactful and constructive. >> Remember that submitting patches for public critique can be very >> intimidating > > > I found this to be true. The tone on the list could at times feel un-helpful > (to the new person). It is almost as if it is an initiation - those on the > list know the protocols, and new folk either arrive like a bull in a china > shop, or more likely, timidly push the patch under the door and run away > (and variations in between) - some never push out their (drafted) patch. Interesting! I've had the opposite opinion. I've often been surprised at how much constructive feedback has been given, and the thoughtfulness of the reviewers to offer up alternative solutions, show examples, etc. Junio, Jeff, and especially Jonathan have been particularly good on that front--at least those are some of the regulars that stick out in my mind. Overall, I've been pretty happy with the community, and while I haven't contributed much, I generally enjoy reading the emails. I feel like I learn something new all the time. :-) -John -- 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] Documentation/CommunityGuidelines
On Tue, Jun 11, 2013 at 1:43 PM, Michael Haggerty wrote: > On 06/11/2013 08:16 PM, Ramkumar Ramachandra wrote: >> This is an exercise. I can easily be more tactful (as evidenced by >> other threads), but I'm choosing not to be. I want you to focus on >> the argument, and not the tone. > > I stopped reading your email here. Yeah, you are ignoring the arguments, what a surprise. > In German there is an expression "Der Ton macht die Musik": "the tone > makes the music", by which they mean that the *way* something is said is > at least as important as what is said. I know you don't care about reality, but you are committing the is-ought fallacy. You are describing the way things are, not the way things should be. Yes, most people can't handle rational arguments, or truth, and need hypocrites to hide things from them. That's why politics is surrounded by people who never say the truth.. not really. That doesn't mean that's the way things _ought_ to be, specially not the way *you* should act. -- 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: What's cooking in git.git (Jun 2013, #04; Tue, 11)
Felipe Contreras writes: > On Tue, Jun 11, 2013 at 5:34 PM, Junio C Hamano wrote: > >> * fc/remote-helpers-use-specified-python (2013-05-28) 4 commits >> - remote-helpers: add exec-path links >> - remote-helpers: allow direct test execution >> - remote-helpers: rename tests >> - remote-helpers: generate scripts >> >> I do not particularly think the second from the bottom is a good >> change, but it takes the remainder of the series hostage. >> >> Waiting for a reroll. > > Waiting for a reroll? This is the first time I hear you are not going > to merge these. > > You say actions have consequences... well, you are right, you can stop > waiting for a reroll. It has been listed in that state in the previous issue #03 as well. And if you do not want to reroll, that is fine. We can discard it for now and the next person who finds the need to can redo the change. Will discard the topic, then. -- 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: What's cooking in git.git (Jun 2013, #04; Tue, 11)
On Tue, Jun 11, 2013 at 5:34 PM, Junio C Hamano wrote: > * fc/remote-helpers-use-specified-python (2013-05-28) 4 commits > - remote-helpers: add exec-path links > - remote-helpers: allow direct test execution > - remote-helpers: rename tests > - remote-helpers: generate scripts > > I do not particularly think the second from the bottom is a good > change, but it takes the remainder of the series hostage. > > Waiting for a reroll. Waiting for a reroll? This is the first time I hear you are not going to merge these. You say actions have consequences... well, you are right, you can stop waiting for a reroll. -- 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] Documentation/CommunityGuidelines
On Tue, Jun 11, 2013 at 3:55 PM, Junio C Hamano wrote: > Jeff King writes: > >> So there are no hard rules, and this is not a democracy[1]. For the most >> part the community runs itself in an open and collective fashion, and >> the dictator's job is easy; but ultimately, he or she is in charge of >> what gets applied and what doesn't. Rules like "break ties in favor of >> reviewers" are just a guideline for the dictator to use in making >> decisions. >> >> I do not think any of that is news to you, but I think the point needs >> to be made, as it applies to any concrete rules. > > My original draft had "I am hoping we do not have to come to that" > after "(I heard some communities break ties this way)", but I > removed it by mistake. > > And I think you are right. I also am hoping that I am being fair to > dictate ;-) Fair? Fairness requires to judge each action without biases, nor double standards. In the case of an open source community it requires you to listen to the arguments before dismissing them, and consider the patches before dropping them on the floor. Fairness requires no favoritism. You think you are being fair? You have acted the equivalent of a judge that says "oh, you again? I don't need to look at the case, you are a drunk and you go to jail". I'm not saying that's wrong, I'm saying you can't call that fair. -- 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/2] [submodule] Replace perl-code with sh
This is a work built on
http://thread.gmane.org/gmane.comp.version-control.git/198873/focus=198930
Basically git-submodule.sh needs to use something else than sh to handle
newline in filenames (and therefore needs to use a language that accepts
\0 in strings).
However, since we're not there yet. I've thrown out the only
perl-dependency for git-submodule.sh. It decreases the number of
lines of code and uses the same solution as the rest of the script
already do.
This would lead to less forks and faster code. A simple testrun of
t7400-submodule-basic.sh before this patch resulted in:
real0m8.359s
user0m8.921s
sys 0m3.888s
real0m9.062s
user0m9.025s
sys 0m3.784s
real0m8.490s
user0m9.065s
sys 0m3.740s
After this patch was applied:
real0m7.417s
user0m8.717s
sys 0m3.804s
real0m7.873s
user0m8.821s
sys 0m3.692s
real0m8.950s
user0m8.765s
sys 0m3.760s
Signed-off-by: Fredrik Gustafsson
---
git-submodule.sh | 52 +++-
1 file changed, 23 insertions(+), 29 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 31524d3..1652781 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -112,39 +112,33 @@ resolve_relative_url ()
#
module_list()
{
+ null_sha1=
+ unmerged=
(
git ls-files --error-unmatch --stage -z -- "$@" ||
- echo "unmatched pathspec exists"
+ echo "#unmatched"
) |
sed -e 's/\x00/\n/g' |
- perl -e '
- my %unmerged = ();
- my ($null_sha1) = ("0" x 40);
- my @out = ();
- my $unmatched = 0;
- while () {
- if (/^unmatched pathspec/) {
- $unmatched = 1;
- next;
- }
- chomp;
- my ($mode, $sha1, $stage, $path) =
- /^([0-7]+) ([0-9a-f]{40}) ([0-3])\t(.*)$/;
- next unless $mode eq "16";
- if ($stage ne "0") {
- if (!$unmerged{$path}++) {
- push @out, "$mode $null_sha1 U\t$path\n";
- }
- next;
- }
- push @out, "$_\n";
- }
- if ($unmatched) {
- print "#unmatched\n";
- } else {
- print for (@out);
- }
- '
+ while read mode sha1 stage path
+ do
+ if test $mode = "#unmatched"
+ then
+ echo "#unmatched"
+ fi
+ if test $mode = "16"
+ then
+ if test $stage != "0"
+ then
+ if test "$unmerged" != "$path"
+ then
+ echo "$mode $null_sha1 U $path"
+ fi
+ unmerged="$path"
+ else
+ echo "$mode $sha1 $stage $path"
+ fi
+ fi
+ done
}
die_if_unmatched ()
--
1.8.3.253.g20b40b5.dirty
--
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 1/2] [submodule] handle multibyte characters in name
Bugg reported here:
http://thread.gmane.org/gmane.comp.version-control.git/218922/focus=226791
Note that newline (\n) is still not supported and will not be until the
sh-script is replaced by something in an other language. This however
let us to use mostly all other strange characters.
Signed-off-by: Fredrik Gustafsson
---
git-submodule.sh | 3 ++-
t/t7400-submodule-basic.sh | 5 +
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 79bfaac..31524d3 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -113,9 +113,10 @@ resolve_relative_url ()
module_list()
{
(
- git ls-files --error-unmatch --stage -- "$@" ||
+ git ls-files --error-unmatch --stage -z -- "$@" ||
echo "unmatched pathspec exists"
) |
+ sed -e 's/\x00/\n/g' |
perl -e '
my %unmerged = ();
my ($null_sha1) = ("0" x 40);
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index ff26535..47ab7e7 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -868,4 +868,9 @@ test_expect_success 'submodule deinit fails when submodule
has a .git directory
test -n "$(git config --get-regexp "submodule\.example\.")"
'
+test_expect_success 'submodule with strange name works "å äö"' '
+ git init "å äö" &&
+ git submodule add "./å äö" &&
+ test -n "$(cat .gitmodules | grep "å äö")"
+'
test_done
--
1.8.3.253.g20b40b5.dirty
--
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 0/2] module_list enhancements
Cleanup and enhanced module_list (see patches for details). All new functionality is in the first patch, the second one deals only with cleanup. I would prefer if both got applied. Fredrik Gustafsson (2): [submodule] handle multibyte characters in name [submodule] Replace perl-code with sh git-submodule.sh | 55 +- t/t7400-submodule-basic.sh | 5 + 2 files changed, 30 insertions(+), 30 deletions(-) -- 1.8.3.253.g20b40b5.dirty -- 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
What's cooking in git.git (Jun 2013, #04; Tue, 11)
Here are the topics that have been cooking. Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.
You can find the changes described here in the integration branches
of the repositories listed at
http://git-blame.blogspot.com/p/git-public-repositories.html
--
[Graduated to "master"]
* ar/wildmatch-foldcase (2013-06-02) 1 commit
(merged to 'next' on 2013-06-04 at 3180bcc)
+ wildmatch: properly fold case everywhere
The wildmatch engine did not honor WM_CASEFOLD option correctly.
* cb/log-follow-with-combined (2013-05-28) 1 commit
(merged to 'next' on 2013-06-04 at d5bf4f3)
+ fix segfault with git log -c --follow
* cm/gitweb-project-list-persistent-cgi-fix (2013-06-07) 1 commit
(merged to 'next' on 2013-06-07 at b16ca1f)
+ gitweb: fix problem causing erroneous project list
"gitweb" forgot to clear a global variable $search_regexp upon each
request, mistakenly carrying over the previous search to a new one
when used as a persistent CGI.
* cr/git-work-tree-sans-git-dir (2013-06-03) 1 commit
(merged to 'next' on 2013-06-04 at bebedca)
+ git.txt: remove stale comment regarding GIT_WORK_TREE
These days, "git --work-tree=there cmd" without specifying an
explicit --git-dir=here will do the usual discovery, but we had a
description of older behaviour in the documentation.
* fc/at-head (2013-05-08) 13 commits
(merged to 'next' on 2013-06-04 at f334a2a)
+ sha1_name: compare variable with constant, not constant with variable
+ Add new @ shortcut for HEAD
+ sha1_name: refactor reinterpret()
+ sha1_name: check @{-N} errors sooner
+ sha1_name: reorganize get_sha1_basic()
+ sha1_name: don't waste cycles in the @-parsing loop
+ sha1_name: remove unnecessary braces
+ sha1_name: remove no-op
+ tests: at-combinations: @{N} versus HEAD@{N}
+ tests: at-combinations: increase coverage
+ tests: at-combinations: improve nonsense()
+ tests: at-combinations: check ref names directly
+ tests: at-combinations: simplify setup
Instead of typing four capital letters "HEAD", you can say "@".
* fc/completion-less-ls-remote (2013-06-02) 1 commit
(merged to 'next' on 2013-06-03 at 6624f0b)
+ completion: avoid ls-remote in certain scenarios
* fc/do-not-use-the-index-in-add-to-index (2013-06-03) 2 commits
(merged to 'next' on 2013-06-04 at 94e7b60)
+ read-cache: trivial style cleanups
+ read-cache: fix wrong 'the_index' usage
* fc/remote-bzr (2013-05-28) 8 commits
(merged to 'next' on 2013-06-04 at a603082)
+ remote-bzr: add fallback check for a partial clone
+ remote-bzr: reorganize the way 'wanted' works
+ remote-bzr: trivial cleanups
+ remote-bzr: change global repo
+ remote-bzr: delay cloning/pulling
+ remote-bzr: simplify get_remote_branch()
+ remote-bzr: fix for files with spaces
+ remote-bzr: recover from failed clones
* fc/remote-hg (2013-05-28) 50 commits
(merged to 'next' on 2013-06-04 at 9ee7dab)
+ remote-hg: add support for --force
+ remote-hg: add support for --dry-run
+ remote-hg: check if a fetch is needed
+ remote-hg: trivial cleanup
+ remote-helpers: improve marks usage
+ remote-hg: add check_push() helper
+ remote-hg: add setup_big_push() helper
+ remote-hg: remove files before modifications
+ remote-hg: improve lightweight tag author
+ remote-hg: use remote 'default' not local one
+ remote-hg: improve branch listing
+ remote-hg: simplify branch_tip()
+ remote-hg: check diverged bookmarks
+ remote-hg: pass around revision refs
+ remote-hg: implement custom checkheads()
+ remote-hg: implement custom push()
+ remote-hg: only update necessary revisions
+ remote-hg: force remote bookmark push selectively
+ remote-hg: reorganize bookmark handling
+ remote-hg: add test for failed double push
+ remote-hg: add test for big push
+ remote-hg: add test for new bookmark special
+ remote-hg: add test for bookmark diverge
+ remote-hg: add test for diverged push
+ remote-hg: add test to push new bookmark
+ remote-hg: add remote tests
+ remote-hg: update bookmarks when using a remote
+ remote-hg: add check_bookmark() test helper
+ remote-bzr: simplify test checks
+ remote-hg: add tests for 'master' bookmark
+ remote-hg: always point HEAD to master
+ remote-hg: improve progress calculation
+ remote-hg: trivial cleanups
+ remote-hg: ensure remote rebasing works
+ remote-hg: upgrade version 1 marks
+ remote-hg: switch from revisions to SHA-1 noteids
+ remote-hg: add version checks to the marks
+ remote-hg: improve node traversing
+ remote-hg: shuffle some code
+ remote-hg: use a shared repository store
+ remote-hg: load all extensions
+ remote-hg: test: simplify previous branch checkout
+ remote-helpers: test: simplify remote URLs
+ remote-helpers: tests: general improvements
+ remote-helpers: test: cleanup style
+ remote-helpers: test: cleanup white-spaces
+ remote-hg: trivial reorganization
+ remote-hg: test: be a
Re: [PATCH v3 21/28] git-remote-mediawiki: Put long code into a subroutine
Oops, forgot to take this into account before sending v4 of my series of
patch. I just noticed that, sorry...
Le 11/06/2013 17:42, Junio C Hamano a écrit :
> I am guessing that the new sub, parse_command, uses a local @cmd and
> this is an attempt to avoid using the same name, but this renaming
> of the variable is not explained.
This is indeed what I intended to do.
> I also wonder if you need this global @cmd/@cmds. Instead of
> passing cmdref, wouldn't it be simpler to have the helper split the
> line, i.e. something like...
>
> sub parse_command {
> my ($line) = @_;
> my @cmd = split(/ /, $line);
> unless (defined @cmd) {
> return 0;
> }
> ... check capabilities, list, etc
> return 1;
> }
>
> while () {
> chomp;
> if (!parse_command($_)) {
> unknown command, aborting
> last;
> }
> }
>
>
--
Célestin Matte
--
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 v4 19/31] git-remote-mediawiki: Check return value of open
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 15ad19b..d95119f 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -337,7 +337,8 @@ sub get_mw_pages {
sub run_git {
my $args = shift;
my $encoding = (shift || "encoding(UTF-8)");
- open(my $git, "-|:$encoding", "git " . $args);
+ open(my $git, "-|:$encoding", "git " . $args)
+ or die "Unable to open: $!\n";
my $res = do {
local $/ = undef;
<$git>
--
1.7.9.5
--
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 v4 10/31] git-remote-mediawiki: Change separator of some regexps
Use {}{} instead of /// when slashes are used inside the regexp so as not to
escape it.
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl |6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 7537305..b01d402 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -120,7 +120,7 @@ chomp($dumb_push);
$dumb_push = ($dumb_push eq "true");
my $wiki_name = $url;
-$wiki_name =~ s/[^\/]*:\/\///;
+$wiki_name =~ s{[^/]*://}{};
# If URL is like http://user:passw...@example.com/, we clearly don't
# want the password in $wiki_name. While we're there, also remove user
# and '@' sign, to avoid author like MWUser@httpu...@host.com
@@ -564,7 +564,7 @@ sub mediawiki_smudge {
sub mediawiki_clean_filename {
my $filename = shift;
- $filename =~ s/@{[SLASH_REPLACEMENT]}/\//g;
+ $filename =~ s{@{[SLASH_REPLACEMENT]}}{/}g;
# [, ], |, {, and } are forbidden by MediaWiki, even URL-encoded.
# Do a variant of URL-encoding, i.e. looks like URL-encoding,
# but with _ added to prevent MediaWiki from thinking this is
@@ -578,7 +578,7 @@ sub mediawiki_clean_filename {
sub mediawiki_smudge_filename {
my $filename = shift;
- $filename =~ s/\//@{[SLASH_REPLACEMENT]}/g;
+ $filename =~ s{/}{@{[SLASH_REPLACEMENT]}}g;
$filename =~ s/ /_/g;
# Decode forbidden characters encoded in mediawiki_clean_filename
$filename =~ s/_%_([0-9a-fA-F][0-9a-fA-F])/sprintf("%c", hex($1))/ge;
--
1.7.9.5
--
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 v4 18/31] git-remote-mediawiki: Assign a variable as undef and make proper indentation
Explicitly assign local variable $/ as undef and make a proper
one-instruction-by-line indentation
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 0610daa..15ad19b 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -338,7 +338,10 @@ sub run_git {
my $args = shift;
my $encoding = (shift || "encoding(UTF-8)");
open(my $git, "-|:$encoding", "git " . $args);
- my $res = do { local $/; <$git> };
+ my $res = do {
+ local $/ = undef;
+ <$git>
+ };
close($git);
return $res;
--
1.7.9.5
--
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 v4 04/31] git-remote-mediawiki: Always end a subroutine with a return
Follow Subroutines::RequireFinalReturn
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl | 18 ++
1 file changed, 18 insertions(+)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 5e198e0..f19b276 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -197,12 +197,14 @@ sub mw_connect_maybe {
exit 1;
}
}
+ return;
}
## Functions for listing pages on the remote wiki
sub get_mw_tracked_pages {
my $pages = shift;
get_mw_page_list(\@tracked_pages, $pages);
+ return;
}
sub get_mw_page_list {
@@ -218,6 +220,7 @@ sub get_mw_page_list {
get_mw_first_pages(\@slice, $pages);
@some_pages = @some_pages[51..$#some_pages];
}
+ return;
}
sub get_mw_tracked_categories {
@@ -240,6 +243,7 @@ sub get_mw_tracked_categories {
$pages->{$page->{title}} = $page;
}
}
+ return;
}
sub get_mw_all_pages {
@@ -259,6 +263,7 @@ sub get_mw_all_pages {
foreach my $page (@{$mw_pages}) {
$pages->{$page->{title}} = $page;
}
+ return;
}
# queries the wiki for a set of pages. Meant to be used within a loop
@@ -289,6 +294,7 @@ sub get_mw_first_pages {
$pages->{$page->{title}} = $page;
}
}
+ return;
}
# Get the list of pages to be fetched according to configuration.
@@ -357,6 +363,7 @@ sub get_all_mediafiles {
foreach my $page (@{$mw_pages}) {
$pages->{$page->{title}} = $page;
}
+ return;
}
sub get_linked_mediafiles {
@@ -403,6 +410,7 @@ sub get_linked_mediafiles {
@titles = @titles[($batch+1)..$#titles];
}
+ return;
}
sub get_mw_mediafile_for_page_revision {
@@ -578,6 +586,7 @@ sub mediawiki_smudge_filename {
sub literal_data {
my ($content) = @_;
print STDOUT "data ", bytes::length($content), "\n", $content;
+ return;
}
sub literal_data_raw {
@@ -588,6 +597,7 @@ sub literal_data_raw {
binmode STDOUT, ":raw";
print STDOUT "data ", bytes::length($content), "\n", $content;
binmode STDOUT, ":encoding(UTF-8)";
+ return;
}
sub mw_capabilities {
@@ -599,6 +609,7 @@ sub mw_capabilities {
print STDOUT "list\n";
print STDOUT "push\n";
print STDOUT "\n";
+ return;
}
sub mw_list {
@@ -607,11 +618,13 @@ sub mw_list {
print STDOUT "? refs/heads/master\n";
print STDOUT "\@refs/heads/master HEAD\n";
print STDOUT "\n";
+ return;
}
sub mw_option {
print STDERR "remote-helper command 'option $_[0]' not yet
implemented\n";
print STDOUT "unsupported\n";
+ return;
}
sub fetch_mw_revisions_for_page {
@@ -733,6 +746,7 @@ sub import_file_revision {
print STDOUT "N inline :$n\n";
literal_data("mediawiki_revision: " . $commit{mw_revision});
print STDOUT "\n\n";
+ return;
}
# parse a sequence of
@@ -753,6 +767,7 @@ sub get_more_refs {
die("Invalid command in a '$cmd' batch: ". $_);
}
}
+ return;
}
sub mw_import {
@@ -762,6 +777,7 @@ sub mw_import {
mw_import_ref($ref);
}
print STDOUT "done\n";
+ return;
}
sub mw_import_ref {
@@ -805,6 +821,7 @@ sub mw_import_ref {
# thrown saying that HEAD is referring to unknown object
000
# and the clone fails.
}
+ return;
}
sub mw_import_ref_by_pages {
@@ -,6 +1128,7 @@ sub mw_push {
print STDERR " git pull --rebase\n";
print STDERR "\n";
}
+ return;
}
sub mw_push_revision {
--
1.7.9.5
--
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 v3 0/4] log --author-date-order
The first one is unchanged. The second one was redone with Peff's help, and the other two patches have been adjusted for it. Adding tests to t4202 and/or t6012 is left as an exercise to readers. Junio C Hamano (4): toposort: rename "lifo" field prio-queue: priority queue of pointers to structs sort-in-topological-order: use prio-queue log: --author-date-order .gitignore | 1 + Documentation/rev-list-options.txt | 4 + Makefile | 3 + builtin/log.c | 2 +- builtin/show-branch.c | 14 ++-- commit.c | 145 ++--- commit.h | 15 +++- prio-queue.c | 84 + prio-queue.h | 48 revision.c | 13 ++-- revision.h | 6 +- t/t0009-prio-queue.sh | 50 + test-prio-queue.c | 39 ++ 13 files changed, 381 insertions(+), 43 deletions(-) create mode 100644 prio-queue.c create mode 100644 prio-queue.h create mode 100755 t/t0009-prio-queue.sh create mode 100644 test-prio-queue.c -- 1.8.3.1-494-g51b8af5 -- 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 v4 09/31] git-remote-mediawiki: Change the behaviour of a split
A "split ' '" is turned into a "split / /", which changes its behaviour: the
old method matched a run of whitespaces (/\s*/), while the new one will match a
single whitespace, which is what we want here. Indeed, in other contexts,
changing split(' ') to split(/ /) could potentially be a regression, however,
here, when parsing the output of "rev-list --parents", whose output SHA-1's are
each separated by a single space, splitting on a single space is perfectly
correct.
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 7ce640f..7537305 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -1168,7 +1168,7 @@ sub mw_push_revision {
my %local_ancestry;
foreach my $line (@local_ancestry) {
if (my ($child, $parents) = $line =~ /^-?([a-f0-9]+)
([a-f0-9 ]+)/) {
- foreach my $parent (split(' ', $parents)) {
+ foreach my $parent (split(/ /, $parents)) {
$local_ancestry{$parent} = $child;
}
} elsif (!$line =~ /^([a-f0-9]+)/) {
--
1.7.9.5
--
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 v4 23/31] git-remote-mediawiki: Brace file handles for print for more clarity
This follows the following rule:
InputOutput::RequireBracedFileHandleWithPrint (Severity: 1)
The `print' and `printf' functions have a unique syntax that supports an
optional file handle argument. Conway suggests wrapping this argument in
braces to make it visually stand out from the other arguments. When you
put braces around any of the special package-level file handles like
`STDOUT', `STDERR', and `DATA', you must the `'*'' sigil or else it
won't compile under `use strict 'subs''.
print $FH "Mary had a little lamb\n"; #not ok
print {$FH} "Mary had a little lamb\n"; #ok
print STDERR $foo, $bar, $baz; #not ok
print {STDERR} $foo, $bar, $baz; #won't compile under 'strict'
print {*STDERR} $foo, $bar, $baz; #perfect!
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl | 202 +--
1 file changed, 101 insertions(+), 101 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index e89bb02..5174080 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -173,7 +173,7 @@ sub parse_command {
} elsif ($cmd[0] eq 'push') {
mw_push($cmd[1]);
} else {
- print STDERR "Unknown command. Aborting...\n";
+ print {*STDERR} "Unknown command. Aborting...\n";
return 0;
}
return 1;
@@ -200,10 +200,10 @@ sub mw_connect_maybe {
lgdomain => $wiki_domain};
if ($mediawiki->login($request)) {
Git::credential \%credential, 'approve';
- print STDERR qq(Logged in mediawiki user
"$credential{username}".\n);
+ print {*STDERR} qq(Logged in mediawiki user
"$credential{username}".\n);
} else {
- print STDERR qq(Failed to log in mediawiki user
"$credential{username}" on ${url}\n);
- print STDERR ' (error ' .
+ print {*STDERR} qq(Failed to log in mediawiki user
"$credential{username}" on ${url}\n);
+ print {*STDERR} ' (error ' .
$mediawiki->{error}->{code} . ': ' .
$mediawiki->{error}->{details} . ")\n";
Git::credential \%credential, 'reject';
@@ -268,9 +268,9 @@ sub get_mw_all_pages {
aplimit => 'max'
});
if (!defined($mw_pages)) {
- print STDERR "fatal: could not get the list of wiki pages.\n";
- print STDERR "fatal: '${url}' does not appear to be a
mediawiki\n";
- print STDERR "fatal: make sure '${url}/api.php' is a valid
page.\n";
+ print {*STDERR} "fatal: could not get the list of wiki
pages.\n";
+ print {*STDERR} "fatal: '${url}' does not appear to be a
mediawiki\n";
+ print {*STDERR} "fatal: make sure '${url}/api.php' is a valid
page.\n";
exit 1;
}
foreach my $page (@{$mw_pages}) {
@@ -295,14 +295,14 @@ sub get_mw_first_pages {
titles => $titles,
});
if (!defined($mw_pages)) {
- print STDERR "fatal: could not query the list of wiki pages.\n";
- print STDERR "fatal: '${url}' does not appear to be a
mediawiki\n";
- print STDERR "fatal: make sure '${url}/api.php' is a valid
page.\n";
+ print {*STDERR} "fatal: could not query the list of wiki
pages.\n";
+ print {*STDERR} "fatal: '${url}' does not appear to be a
mediawiki\n";
+ print {*STDERR} "fatal: make sure '${url}/api.php' is a valid
page.\n";
exit 1;
}
while (my ($id, $page) = each(%{$mw_pages->{query}->{pages}})) {
if ($id < 0) {
- print STDERR "Warning: page $page->{title} not found on
wiki\n";
+ print {*STDERR} "Warning: page $page->{title} not found
on wiki\n";
} else {
$pages->{$page->{title}} = $page;
}
@@ -314,7 +314,7 @@ sub get_mw_first_pages {
sub get_mw_pages {
mw_connect_maybe();
- print STDERR "Listing pages on remote wiki...\n";
+ print {*STDERR} "Listing pages on remote wiki...\n";
my %pages; # hash on page titles to avoid duplicates
my $user_defined;
@@ -332,14 +332,14 @@ sub get_mw_pages {
get_mw_all_pages(\%pages);
}
if ($import_media) {
- print STDERR "Getting media files for selected pages...\n";
+ print {*STDERR} "Getting media files for selected pages...\n";
if ($user_defined) {
get_linked_mediafiles(\%pages);
} else {
[PATCH v4 11/31] git-remote-mediawiki: Change style in a regexp
In this regexp, ' |\n' is used, whereas its equivalent '[ \n]', which is
clearer, is used elsewhere. Make the style coherent.
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index b01d402..267c164 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -1192,7 +1192,7 @@ sub mw_push_revision {
my @history = split(/\n/, $history);
@history = @history[1..$#history];
foreach my $line (reverse @history) {
- my @commit_info_split = split(/ |\n/, $line);
+ my @commit_info_split = split(/[ \n]/, $line);
push(@commit_pairs, \@commit_info_split);
}
}
--
1.7.9.5
--
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 v3 3/4] sort-in-topological-order: use prio-queue
Use the prio-queue data structure to implement a priority queue of
commits sorted by committer date, when handling --date-order. The
structure can also be used as a simple LIFO stack, which is a good
match for --topo-order processing.
Signed-off-by: Junio C Hamano
---
commit.c | 75 +++-
prio-queue.c | 13 +++
prio-queue.h | 3 +++
3 files changed, 60 insertions(+), 31 deletions(-)
diff --git a/commit.c b/commit.c
index 11b9635..8b84ebf 100644
--- a/commit.c
+++ b/commit.c
@@ -9,6 +9,7 @@
#include "gpg-interface.h"
#include "mergesort.h"
#include "commit-slab.h"
+#include "prio-queue.h"
static struct commit_extra_header *read_commit_extra_header_lines(const char
*buf, size_t len, const char **);
@@ -509,21 +510,42 @@ struct commit *pop_commit(struct commit_list **stack)
/* count number of children that have not been emitted */
define_commit_slab(indegree_slab, int);
+static int compare_commits_by_commit_date(const void *a_, const void *b_, void
*unused)
+{
+ const struct commit *a = a_, *b = b_;
+ /* newer commits with larger date first */
+ if (a->date < b->date)
+ return 1;
+ else if (a->date > b->date)
+ return -1;
+ return 0;
+}
+
/*
* Performs an in-place topological sort on the list supplied.
*/
-void sort_in_topological_order(struct commit_list ** list, enum rev_sort_order
sort_order)
+void sort_in_topological_order(struct commit_list **list, enum rev_sort_order
sort_order)
{
struct commit_list *next, *orig = *list;
- struct commit_list *work, **insert;
struct commit_list **pptr;
struct indegree_slab indegree;
+ struct prio_queue queue;
+ struct commit *commit;
if (!orig)
return;
*list = NULL;
init_indegree_slab(&indegree);
+ memset(&queue, '\0', sizeof(queue));
+ switch (sort_order) {
+ default: /* REV_SORT_IN_GRAPH_ORDER */
+ queue.compare = NULL;
+ break;
+ case REV_SORT_BY_COMMIT_DATE:
+ queue.compare = compare_commits_by_commit_date;
+ break;
+ }
/* Mark them and clear the indegree */
for (next = orig; next; next = next->next) {
@@ -533,7 +555,7 @@ void sort_in_topological_order(struct commit_list ** list,
enum rev_sort_order s
/* update the indegree */
for (next = orig; next; next = next->next) {
- struct commit_list * parents = next->item->parents;
+ struct commit_list *parents = next->item->parents;
while (parents) {
struct commit *parent = parents->item;
int *pi = indegree_slab_at(&indegree, parent);
@@ -551,30 +573,28 @@ void sort_in_topological_order(struct commit_list **
list, enum rev_sort_order s
*
* the tips serve as a starting set for the work queue.
*/
- work = NULL;
- insert = &work;
for (next = orig; next; next = next->next) {
struct commit *commit = next->item;
if (*(indegree_slab_at(&indegree, commit)) == 1)
- insert = &commit_list_insert(commit, insert)->next;
+ prio_queue_put(&queue, commit);
}
- /* process the list in topological order */
- if (sort_order != REV_SORT_IN_GRAPH_ORDER)
- commit_list_sort_by_date(&work);
+ /*
+* This is unfortunate; the initial tips need to be shown
+* in the order given from the revision traversal machinery.
+*/
+ if (sort_order == REV_SORT_IN_GRAPH_ORDER)
+ prio_queue_reverse(&queue);
+
+ /* We no longer need the commit list */
+ free_commit_list(orig);
pptr = list;
*list = NULL;
- while (work) {
- struct commit *commit;
- struct commit_list *parents, *work_item;
-
- work_item = work;
- work = work_item->next;
- work_item->next = NULL;
+ while ((commit = prio_queue_get(&queue)) != NULL) {
+ struct commit_list *parents;
- commit = work_item->item;
for (parents = commit->parents; parents ; parents =
parents->next) {
struct commit *parent = parents->item;
int *pi = indegree_slab_at(&indegree, parent);
@@ -587,27 +607,20 @@ void sort_in_topological_order(struct commit_list **
list, enum rev_sort_order s
* when all their children have been emitted thereby
* guaranteeing topological order.
*/
- if (--(*pi) == 1) {
- switch (sort_order) {
- case REV_SORT_BY_COMMIT_DATE:
- commit_list_insert_by_date(par
[PATCH v4 20/31] git-remote-mediawiki: remove import of unused open2
Signed-off-by: Célestin Matte Signed-off-by: Matthieu Moy --- contrib/mw-to-git/git-remote-mediawiki.perl |1 - 1 file changed, 1 deletion(-) diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl index d95119f..7acbec8 100755 --- a/contrib/mw-to-git/git-remote-mediawiki.perl +++ b/contrib/mw-to-git/git-remote-mediawiki.perl @@ -22,7 +22,6 @@ binmode STDERR, ":encoding(UTF-8)"; binmode STDOUT, ":encoding(UTF-8)"; use URI::Escape; -use IPC::Open2; # Mediawiki filenames can contain forward slashes. This variable decides by which pattern they should be replaced use constant SLASH_REPLACEMENT => "%2F"; -- 1.7.9.5 -- 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 v4 14/31] git-remote-mediawiki: Change the name of a variable
Local variable $url has the same name as a global variable. Changing the name
of the local variable prevents future possible misunderstanding.
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl |6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 44c5e0e..63d1530 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -454,14 +454,14 @@ sub get_mw_mediafile_for_page_revision {
}
sub download_mw_mediafile {
- my $url = shift;
+ my $download_url = shift;
- my $response = $mediawiki->{ua}->get($url);
+ my $response = $mediawiki->{ua}->get($download_url);
if ($response->code == 200) {
return $response->decoded_content;
} else {
print STDERR "Error downloading mediafile from :\n";
- print STDERR "URL: $url\n";
+ print STDERR "URL: $download_url\n";
print STDERR "Server response: " . $response->code . " " .
$response->message . "\n";
exit 1;
}
--
1.7.9.5
--
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 v4 17/31] git-remote-mediawiki: Rename a variable ($last) which has the name of a keyword
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl |8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index ef9e60a..0610daa 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -214,11 +214,11 @@ sub get_mw_page_list {
my $pages = shift;
my @some_pages = @$page_list;
while (@some_pages) {
- my $last = 50;
- if ($#some_pages < $last) {
- $last = $#some_pages;
+ my $last_page = 50;
+ if ($#some_pages < $last_page) {
+ $last_page = $#some_pages;
}
- my @slice = @some_pages[0..$last];
+ my @slice = @some_pages[0..$last_page];
get_mw_first_pages(\@slice, $pages);
@some_pages = @some_pages[51..$#some_pages];
}
--
1.7.9.5
--
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 v4 01/31] git-remote-mediawiki: Make a regexp clearer
Perl's split function takes a regex pattern argument. You can also
feed it an expression, which is then compiled into a regex at runtime.
It therefore works to pass your pattern via single quotes, but it is
much less obvious to a reader that the argument is meant to be a
regex, not a static string. Using the traditional slash-delimiters
makes this easier to read.
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 410eae9..a7bb397 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -1170,7 +1170,7 @@ sub mw_push_revision {
# history (linearized with --first-parent)
print STDERR "Warning: no common ancestor, pushing complete
history\n";
my $history = run_git("rev-list --first-parent --children
$local");
- my @history = split('\n', $history);
+ my @history = split(/\n/, $history);
@history = @history[1..$#history];
foreach my $line (reverse @history) {
my @commit_info_split = split(/ |\n/, $line);
--
1.7.9.5
--
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 v4 21/31] git-remote-mediawiki: Put long code into a subroutine
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl | 50 +--
1 file changed, 31 insertions(+), 19 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 7acbec8..da022af 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -126,28 +126,13 @@ $wiki_name =~ s{[^/]*://}{};
$wiki_name =~ s/^.*@//;
# Commands parser
-my @cmd;
+my @cmds;
while () {
chomp;
- @cmd = split(/ /);
- if (defined($cmd[0])) {
+ @cmds = split(/ /);
+ if (defined($cmds[0])) {
# Line not blank
- if ($cmd[0] eq "capabilities") {
- die("Too many arguments for capabilities\n") if
(defined($cmd[1]));
- mw_capabilities();
- } elsif ($cmd[0] eq "list") {
- die("Too many arguments for list\n") if
(defined($cmd[2]));
- mw_list($cmd[1]);
- } elsif ($cmd[0] eq "import") {
- die("Invalid arguments for import\n") if ($cmd[1] eq ""
|| defined($cmd[2]));
- mw_import($cmd[1]);
- } elsif ($cmd[0] eq "option") {
- die("Too many arguments for option\n") if ($cmd[1] eq
"" || $cmd[2] eq "" || defined($cmd[3]));
- mw_option($cmd[1],$cmd[2]);
- } elsif ($cmd[0] eq "push") {
- mw_push($cmd[1]);
- } else {
- print STDERR "Unknown command. Aborting...\n";
+ if (!parse_command(\@cmds)) {
last;
}
} else {
@@ -167,6 +152,33 @@ sub exit_error_usage {
die "ERROR: git-remote-mediawiki module was not called with a correct
number of parameters\n";
}
+sub parse_command {
+ my $cmdref = shift;
+ my @cmd = @{$cmdref};
+ if ($cmd[0] eq "capabilities") {
+ die("Too many arguments for capabilities\n")
+ if (defined($cmd[1]));
+ mw_capabilities();
+ } elsif ($cmd[0] eq "list") {
+ die("Too many arguments for list\n") if (defined($cmd[2]));
+ mw_list($cmd[1]);
+ } elsif ($cmd[0] eq "import") {
+ die("Invalid arguments for import\n")
+ if ($cmd[1] eq "" || defined($cmd[2]));
+ mw_import($cmd[1]);
+ } elsif ($cmd[0] eq "option") {
+ die("Too many arguments for option\n")
+ if ($cmd[1] eq "" || $cmd[2] eq "" || defined($cmd[3]));
+ mw_option($cmd[1],$cmd[2]);
+ } elsif ($cmd[0] eq "push") {
+ mw_push($cmd[1]);
+ } else {
+ print STDERR "Unknown command. Aborting...\n";
+ return 0;
+ }
+ return 1;
+}
+
# MediaWiki API instance, created lazily.
my $mediawiki;
--
1.7.9.5
--
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 v4 06/31] git-remote-mediawiki: Change syntax of map calls
Put first parameter of map inside a block, for better readability.
Follow BuiltinFunctions::RequireBlockMap
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index fe1343d..431e063 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -371,7 +371,7 @@ sub get_all_mediafiles {
sub get_linked_mediafiles {
my $pages = shift;
- my @titles = map $_->{title}, values(%{$pages});
+ my @titles = map { $_->{title} } values(%{$pages});
# The query is split in small batches because of the MW API limit of
# the number of links to be returned (500 links max).
@@ -399,11 +399,13 @@ sub get_linked_mediafiles {
while (my ($id, $page) = each(%{$result->{query}->{pages}})) {
my @media_titles;
if (defined($page->{links})) {
- my @link_titles = map $_->{title},
@{$page->{links}};
+ my @link_titles
+ = map { $_->{title} } @{$page->{links}};
push(@media_titles, @link_titles);
}
if (defined($page->{images})) {
- my @image_titles = map $_->{title},
@{$page->{images}};
+ my @image_titles
+ = map { $_->{title} } @{$page->{images}};
push(@media_titles, @image_titles);
}
if (@media_titles) {
@@ -833,7 +835,7 @@ sub mw_import_ref_by_pages {
my ($n, @revisions) = fetch_mw_revisions(\@pages, $fetch_from);
@revisions = sort {$a->{revid} <=> $b->{revid}} @revisions;
- my @revision_ids = map $_->{revid}, @revisions;
+ my @revision_ids = map { $_->{revid} } @revisions;
return mw_import_revids($fetch_from, \@revision_ids, \%pages_hash);
}
@@ -1246,8 +1248,8 @@ sub get_allowed_file_extensions {
siprop => 'fileextensions'
};
my $result = $mediawiki->api($query);
- my @file_extensions= map
$_->{ext},@{$result->{query}->{fileextensions}};
- my %hashFile = map {$_ => 1}@file_extensions;
+ my @file_extensions = map { $_->{ext}}
@{$result->{query}->{fileextensions}};
+ my %hashFile = map { $_ => 1 } @file_extensions;
return %hashFile;
}
--
1.7.9.5
--
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 v4 16/31] git-remote-mediawiki: Remove unused variable $entry
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl |1 -
1 file changed, 1 deletion(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 6024791..ef9e60a 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -127,7 +127,6 @@ $wiki_name =~ s{[^/]*://}{};
$wiki_name =~ s/^.*@//;
# Commands parser
-my $entry;
my @cmd;
while () {
chomp;
--
1.7.9.5
--
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 v4 15/31] git-remote-mediawiki: Turn double-negated expressions into simple expressions
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl |8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 63d1530..6024791 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -135,16 +135,16 @@ while () {
if (defined($cmd[0])) {
# Line not blank
if ($cmd[0] eq "capabilities") {
- die("Too many arguments for capabilities\n") unless
(!defined($cmd[1]));
+ die("Too many arguments for capabilities\n") if
(defined($cmd[1]));
mw_capabilities();
} elsif ($cmd[0] eq "list") {
- die("Too many arguments for list\n") unless
(!defined($cmd[2]));
+ die("Too many arguments for list\n") if
(defined($cmd[2]));
mw_list($cmd[1]);
} elsif ($cmd[0] eq "import") {
- die("Invalid arguments for import\n") unless ($cmd[1]
ne "" && !defined($cmd[2]));
+ die("Invalid arguments for import\n") if ($cmd[1] eq ""
|| defined($cmd[2]));
mw_import($cmd[1]);
} elsif ($cmd[0] eq "option") {
- die("Too many arguments for option\n") unless ($cmd[1]
ne "" && $cmd[2] ne "" && !defined($cmd[3]));
+ die("Too many arguments for option\n") if ($cmd[1] eq
"" || $cmd[2] eq "" || defined($cmd[3]));
mw_option($cmd[1],$cmd[2]);
} elsif ($cmd[0] eq "push") {
mw_push($cmd[1]);
--
1.7.9.5
--
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 v4 13/31] git-remote-mediawiki: Add newline in the end of die() error messages
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl | 26 +-
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 86229a1..44c5e0e 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -135,16 +135,16 @@ while () {
if (defined($cmd[0])) {
# Line not blank
if ($cmd[0] eq "capabilities") {
- die("Too many arguments for capabilities") unless
(!defined($cmd[1]));
+ die("Too many arguments for capabilities\n") unless
(!defined($cmd[1]));
mw_capabilities();
} elsif ($cmd[0] eq "list") {
- die("Too many arguments for list") unless
(!defined($cmd[2]));
+ die("Too many arguments for list\n") unless
(!defined($cmd[2]));
mw_list($cmd[1]);
} elsif ($cmd[0] eq "import") {
- die("Invalid arguments for import") unless ($cmd[1] ne
"" && !defined($cmd[2]));
+ die("Invalid arguments for import\n") unless ($cmd[1]
ne "" && !defined($cmd[2]));
mw_import($cmd[1]);
} elsif ($cmd[0] eq "option") {
- die("Too many arguments for option") unless ($cmd[1] ne
"" && $cmd[2] ne "" && !defined($cmd[3]));
+ die("Too many arguments for option\n") unless ($cmd[1]
ne "" && $cmd[2] ne "" && !defined($cmd[3]));
mw_option($cmd[1],$cmd[2]);
} elsif ($cmd[0] eq "push") {
mw_push($cmd[1]);
@@ -241,7 +241,7 @@ sub get_mw_tracked_categories {
cmtitle => $category,
cmlimit => 'max' } )
|| die $mediawiki->{error}->{code} . ': '
- . $mediawiki->{error}->{details};
+ . $mediawiki->{error}->{details} . "\n";
foreach my $page (@{$mw_pages}) {
$pages->{$page->{title}} = $page;
}
@@ -766,7 +766,7 @@ sub get_more_refs {
} elsif ($line eq "\n") {
return @refs;
} else {
- die("Invalid command in a '$cmd' batch: ". $_);
+ die("Invalid command in a '$cmd' batch: $_\n");
}
}
return;
@@ -878,7 +878,7 @@ sub mw_import_revids {
my $result = $mediawiki->api($query);
if (!$result) {
- die "Failed to retrieve modified page for revision
$pagerevid";
+ die "Failed to retrieve modified page for revision
$pagerevid\n";
}
if (defined($result->{query}->{badrevids}->{$pagerevid})) {
@@ -887,7 +887,7 @@ sub mw_import_revids {
}
if (!defined($result->{query}->{pages})) {
- die "Invalid revision $pagerevid.";
+ die "Invalid revision $pagerevid.\n";
}
my @result_pages = values(%{$result->{query}->{pages}});
@@ -998,7 +998,7 @@ sub mw_upload_file {
}, {
skip_encoding => 1
} ) || die $mediawiki->{error}->{code} . ':'
-. $mediawiki->{error}->{details};
+. $mediawiki->{error}->{details} . "\n";
my $last_file_page = $mediawiki->get_page({title =>
$path});
$newrevid = $last_file_page->{revid};
print STDERR "Pushed file: $new_sha1 -
$complete_file_name.\n";
@@ -1078,7 +1078,7 @@ sub mw_push_file {
# Other errors. Shouldn't happen => just die()
die 'Fatal: Error ' .
$mediawiki->{error}->{code} .
- ' from mediwiki: ' .
$mediawiki->{error}->{details};
+ ' from mediwiki: ' .
$mediawiki->{error}->{details} . "\n";
}
}
$newrevid = $result->{edit}->{newrevid};
@@ -1100,7 +1100,7 @@ sub mw_push {
my $pushed;
for my $refspec (@refsspecs) {
my ($force, $local, $remote) = $refspec =~
/^(\+)?([^:]*):([^:]*)$/
- or die("Invalid refspec for push. Expected : or
+:");
+ or die("Invalid refspec for push. Expected : or
+:\n");
if ($force) {
print STDERR "Warning: forced push not allowed on a
MediaWiki.\n";
}
@@ -1172,7 +1172,7 @@ sub mw_push_revision {
[PATCH v4 02/31] git-remote-mediawiki: Move "use warnings;" before any instruction
Signed-off-by: Célestin Matte Signed-off-by: Matthieu Moy --- contrib/mw-to-git/git-remote-mediawiki.perl |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl index a7bb397..863ecc9 100755 --- a/contrib/mw-to-git/git-remote-mediawiki.perl +++ b/contrib/mw-to-git/git-remote-mediawiki.perl @@ -15,6 +15,7 @@ use strict; use MediaWiki::API; use Git; use DateTime::Format::ISO8601; +use warnings; # By default, use UTF-8 to communicate with Git and the user binmode STDERR, ":utf8"; @@ -23,8 +24,6 @@ binmode STDOUT, ":utf8"; use URI::Escape; use IPC::Open2; -use warnings; - # Mediawiki filenames can contain forward slashes. This variable decides by which pattern they should be replaced use constant SLASH_REPLACEMENT => "%2F"; -- 1.7.9.5 -- 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 v4 07/31] git-remote-mediawiki: Rewrite unclear line of instructions
Subroutines' parameters should be assigned to variable before doing anything
else
Besides, existing instruction affected a variable inside a "if", which break
Git's coding style
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 431e063..2db6467 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -1333,7 +1333,8 @@ sub get_mw_namespace_id {
}
sub get_mw_namespace_id_for_page {
- if (my ($namespace) = $_[0] =~ /^([^:]*):/) {
+ my $namespace = shift;
+ if ($namespace =~ /^([^:]*):/) {
return get_mw_namespace_id($namespace);
} else {
return;
--
1.7.9.5
--
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 v4 05/31] git-remote-mediawiki: Move a variable declaration at the top of the code
%basetimestamps declaration was lost in the middle of subroutines
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl |6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index f19b276..fe1343d 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -95,6 +95,9 @@ unless ($fetch_strategy) {
$fetch_strategy = "by_page";
}
+# Remember the timestamp corresponding to a revision id.
+my %basetimestamps;
+
# Dumb push: don't update notes and mediawiki ref to reflect the last push.
#
# Configurable with mediawiki.dumbPush, or per-remote with
@@ -480,9 +483,6 @@ sub get_last_local_revision {
return $lastrevision_number;
}
-# Remember the timestamp corresponding to a revision id.
-my %basetimestamps;
-
# Get the last remote revision without taking in account which pages are
# tracked or not. This function makes a single request to the wiki thus
# avoid a loop onto all tracked pages. This is useful for the fetch-by-rev
--
1.7.9.5
--
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 v4 08/31] git-remote-mediawiki: Remove useless regexp modifier (m)
m// and // is used randomly. It is better to use the m modifier only when
needed, e.g., when the regexp uses another separator than //.
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl |6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 2db6467..7ce640f 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -761,7 +761,7 @@ sub get_more_refs {
my @refs;
while (1) {
my $line = ;
- if ($line =~ m/^$cmd (.*)$/) {
+ if ($line =~ /^$cmd (.*)$/) {
push(@refs, $1);
} elsif ($line eq "\n") {
return @refs;
@@ -1167,11 +1167,11 @@ sub mw_push_revision {
my @local_ancestry = split(/\n/, run_git("rev-list --boundary
--parents $local ^$parsed_sha1"));
my %local_ancestry;
foreach my $line (@local_ancestry) {
- if (my ($child, $parents) = $line =~ m/^-?([a-f0-9]+)
([a-f0-9 ]+)/) {
+ if (my ($child, $parents) = $line =~ /^-?([a-f0-9]+)
([a-f0-9 ]+)/) {
foreach my $parent (split(' ', $parents)) {
$local_ancestry{$parent} = $child;
}
- } elsif (!$line =~ m/^([a-f0-9]+)/) {
+ } elsif (!$line =~ /^([a-f0-9]+)/) {
die "Unexpected output from git rev-list:
$line";
}
}
--
1.7.9.5
--
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 v4 31/31] git-remote-mediawiki: Make error message more precise
In subroutine parse_command, error messages were not correct. For the "import"
function, having too much or incorrect arguments displayed both
"invalid arguments", while it displayed "too many arguments" for the "option"
functions under the same conditions.
Separate the two error messages in both cases.
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 5e1fee7..ab221ab 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -175,12 +175,16 @@ sub parse_command {
die("Too many arguments for list\n") if (defined($cmd[2]));
mw_list($cmd[1]);
} elsif ($cmd[0] eq 'import') {
- die("Invalid arguments for import\n")
- if ($cmd[1] eq EMPTY || defined($cmd[2]));
+ die("Invalid argument for import\n")
+ if ($cmd[1] eq EMPTY);
+ die("Too many arguments for import\n")
+ if (defined($cmd[2]));
mw_import($cmd[1]);
} elsif ($cmd[0] eq 'option') {
+ die("Invalid arguments for option\n")
+ if ($cmd[1] eq EMPTY || $cmd[2] eq EMPTY);
die("Too many arguments for option\n")
- if ($cmd[1] eq EMPTY || $cmd[2] eq EMPTY ||
defined($cmd[3]));
+ if (defined($cmd[3]));
mw_option($cmd[1],$cmd[2]);
} elsif ($cmd[0] eq 'push') {
mw_push($cmd[1]);
--
1.7.9.5
--
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 v4 25/31] git-remote-mediawiki: Don't use quotes for empty strings
Empty strings are replaced by an $EMPTY constant.
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl | 18 ++
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 4764be5..0bf9a0d 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -40,6 +40,8 @@ use constant NULL_SHA1 =>
'';
# Used on Git's side to reflect empty edit messages on the wiki
use constant EMPTY_MESSAGE => '*Empty MediaWiki Message*';
+use constant EMPTY => q{};
+
if (@ARGV != 2) {
exit_error_usage();
}
@@ -164,11 +166,11 @@ sub parse_command {
mw_list($cmd[1]);
} elsif ($cmd[0] eq 'import') {
die("Invalid arguments for import\n")
- if ($cmd[1] eq "" || defined($cmd[2]));
+ if ($cmd[1] eq EMPTY || defined($cmd[2]));
mw_import($cmd[1]);
} elsif ($cmd[0] eq 'option') {
die("Too many arguments for option\n")
- if ($cmd[1] eq "" || $cmd[2] eq "" || defined($cmd[3]));
+ if ($cmd[1] eq EMPTY || $cmd[2] eq EMPTY ||
defined($cmd[3]));
mw_option($cmd[1],$cmd[2]);
} elsif ($cmd[0] eq 'push') {
mw_push($cmd[1]);
@@ -559,7 +561,7 @@ sub mediawiki_clean {
# Mediawiki does not allow blank space at the end of a page and ends
with a single \n.
# This function right trims a string and adds a \n at the end to follow
this rule
$string =~ s/\s+$//;
- if ($string eq "" && $page_created) {
+ if ($string eq EMPTY && $page_created) {
# Creating empty pages is forbidden.
$string = EMPTY_CONTENT;
}
@@ -570,7 +572,7 @@ sub mediawiki_clean {
sub mediawiki_smudge {
my $string = shift;
if ($string eq EMPTY_CONTENT) {
- $string = "";
+ $string = EMPTY;
}
# This \n is important. This is due to mediawiki's way to handle end of
files.
return "${string}\n";
@@ -996,7 +998,7 @@ sub mw_upload_file {
} else {
# Don't let perl try to interpret file content as UTF-8 => use
"raw"
my $content = run_git("cat-file blob ${new_sha1}", 'raw');
- if ($content ne "") {
+ if ($content ne EMPTY) {
mw_connect_maybe();
$mediawiki->{config}->{upload_url} =
"${url}/index.php/Special:Upload";
@@ -1038,7 +1040,7 @@ sub mw_push_file {
my $newrevid;
if ($summary eq EMPTY_MESSAGE) {
- $summary = '';
+ $summary = EMPTY;
}
my $new_sha1 = $diff_info_split[3];
@@ -1049,7 +1051,7 @@ sub mw_push_file {
my ($title, $extension) = $complete_file_name =~ /^(.*)\.([^\.]*)$/;
if (!defined($extension)) {
- $extension = "";
+ $extension = EMPTY;
}
if ($extension eq 'mw') {
my $ns = get_mw_namespace_id_for_page($complete_file_name);
@@ -1117,7 +1119,7 @@ sub mw_push {
if ($force) {
print {*STDERR} "Warning: forced push not allowed on a
MediaWiki.\n";
}
- if ($local eq "") {
+ if ($local eq EMPTY) {
print {*STDERR} "Cannot delete remote branch on a
MediaWiki\n";
print {*STDOUT} "error ${remote} cannot delete\n";
next;
--
1.7.9.5
--
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 v3 4/4] log: --author-date-order
Sometimes people would want to view the commits in parallel
histories in the order of author dates, not committer dates.
Teach "topo-order" sort machinery to do so, using a commit-info slab
to record the author dates of each commit, and prio-queue to sort
them.
Signed-off-by: Junio C Hamano
---
* This re-reads the commit object when commit->buf has already been
freed, which is necessary to sort by the author date.
Documentation/rev-list-options.txt | 4 +++
commit.c | 74 ++
commit.h | 3 +-
revision.c | 3 ++
4 files changed, 83 insertions(+), 1 deletion(-)
diff --git a/Documentation/rev-list-options.txt
b/Documentation/rev-list-options.txt
index 3bdbf5e..8302402 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -617,6 +617,10 @@ By default, the commits are shown in reverse chronological
order.
Show no parents before all of its children are shown, but
otherwise show commits in the commit timestamp order.
+--author-date-order::
+ Show no parents before all of its children are shown, but
+ otherwise show commits in the author timestamp order.
+
--topo-order::
Show no parents before all of its children are shown, and
avoid showing commits on multiple lines of history
diff --git a/commit.c b/commit.c
index 8b84ebf..076c1fa 100644
--- a/commit.c
+++ b/commit.c
@@ -510,6 +510,68 @@ struct commit *pop_commit(struct commit_list **stack)
/* count number of children that have not been emitted */
define_commit_slab(indegree_slab, int);
+/* record author-date for each commit object */
+define_commit_slab(author_date_slab, unsigned long);
+
+static void record_author_date(struct author_date_slab *author_date,
+ struct commit *commit)
+{
+ const char *buf, *line_end;
+ char *buffer = NULL;
+ struct ident_split ident;
+ char *date_end;
+ unsigned long date;
+
+ if (!commit->buffer) {
+ unsigned long size;
+ enum object_type type;
+ buffer = read_sha1_file(commit->object.sha1, &type, &size);
+ if (!buffer)
+ return;
+ }
+
+ for (buf = commit->buffer ? commit->buffer : buffer;
+buf;
+buf = line_end + 1) {
+ line_end = strchrnul(buf, '\n');
+ if (prefixcmp(buf, "author ")) {
+ if (!line_end[0] || line_end[1] == '\n')
+ return; /* end of header */
+ continue;
+ }
+ if (split_ident_line(&ident,
+buf + strlen("author "),
+line_end - (buf + strlen("author "))) ||
+ !ident.date_begin || !ident.date_end)
+ goto fail_exit; /* malformed "author" line */
+ break;
+ }
+
+ date = strtoul(ident.date_begin, &date_end, 10);
+ if (date_end != ident.date_end)
+ goto fail_exit; /* malformed date */
+ *(author_date_slab_at(author_date, commit)) = date;
+
+fail_exit:
+ free(buffer);
+}
+
+static int compare_commits_by_author_date(const void *a_, const void *b_,
+ void *cb_data)
+{
+ const struct commit *a = a_, *b = b_;
+ struct author_date_slab *author_date = cb_data;
+ unsigned long a_date = *(author_date_slab_at(author_date, a));
+ unsigned long b_date = *(author_date_slab_at(author_date, b));
+
+ /* newer commits with larger date first */
+ if (a_date < b_date)
+ return 1;
+ else if (a_date > b_date)
+ return -1;
+ return 0;
+}
+
static int compare_commits_by_commit_date(const void *a_, const void *b_, void
*unused)
{
const struct commit *a = a_, *b = b_;
@@ -531,6 +593,7 @@ void sort_in_topological_order(struct commit_list **list,
enum rev_sort_order so
struct indegree_slab indegree;
struct prio_queue queue;
struct commit *commit;
+ struct author_date_slab author_date;
if (!orig)
return;
@@ -538,6 +601,7 @@ void sort_in_topological_order(struct commit_list **list,
enum rev_sort_order so
init_indegree_slab(&indegree);
memset(&queue, '\0', sizeof(queue));
+
switch (sort_order) {
default: /* REV_SORT_IN_GRAPH_ORDER */
queue.compare = NULL;
@@ -545,12 +609,20 @@ void sort_in_topological_order(struct commit_list **list,
enum rev_sort_order so
case REV_SORT_BY_COMMIT_DATE:
queue.compare = compare_commits_by_commit_date;
break;
+ case REV_SORT_BY_AUTHOR_DATE:
+ init_author_date_slab(&author_date);
+ queue.compare = compare_commits_by_author_date;
+
[PATCH v4 03/31] git-remote-mediawiki: Replace :utf8 by :encoding(UTF-8)
Follow perlcritic's InputOutput::RequireEncodingWithUTF8Layer policy
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl |6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 863ecc9..5e198e0 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -18,8 +18,8 @@ use DateTime::Format::ISO8601;
use warnings;
# By default, use UTF-8 to communicate with Git and the user
-binmode STDERR, ":utf8";
-binmode STDOUT, ":utf8";
+binmode STDERR, ":encoding(UTF-8)";
+binmode STDOUT, ":encoding(UTF-8)";
use URI::Escape;
use IPC::Open2;
@@ -587,7 +587,7 @@ sub literal_data_raw {
utf8::downgrade($content);
binmode STDOUT, ":raw";
print STDOUT "data ", bytes::length($content), "\n", $content;
- binmode STDOUT, ":utf8";
+ binmode STDOUT, ":encoding(UTF-8)";
}
sub mw_capabilities {
--
1.7.9.5
--
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 v4 28/31] git-remote-mediawiki: Clearly rewrite double dereference
@$var structures are re-written in the following way: @{$var}
It makes them more readable.
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl |8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 6b6adf2..5e1fee7 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -235,7 +235,7 @@ sub get_mw_tracked_pages {
sub get_mw_page_list {
my $page_list = shift;
my $pages = shift;
- my @some_pages = @$page_list;
+ my @some_pages = @{$page_list};
while (@some_pages) {
my $last_page = SLICE_SIZE;
if ($#some_pages < $last_page) {
@@ -885,7 +885,7 @@ sub mw_import_revids {
my $n_actual = 0;
my $last_timestamp = 0; # Placeholer in case $rev->timestamp is
undefined
- foreach my $pagerevid (@$revision_ids) {
+ foreach my $pagerevid (@{$revision_ids}) {
# Count page even if we skip it, since we display
# $n/$total and $total includes skipped pages.
$n++;
@@ -920,7 +920,7 @@ sub mw_import_revids {
my $page_title = $result_page->{title};
if (!exists($pages->{$page_title})) {
- print {*STDERR} "${n}/", scalar(@$revision_ids),
+ print {*STDERR} "${n}/", scalar(@{$revision_ids}),
": Skipping revision #$rev->{revid} of
${page_title}\n";
next;
}
@@ -953,7 +953,7 @@ sub mw_import_revids {
# If this is a revision of the media page for new version
# of a file do one common commit for both file and media page.
# Else do commit only for that page.
- print {*STDERR} "${n}/", scalar(@$revision_ids), ": Revision
#$rev->{revid} of $commit{title}\n";
+ print {*STDERR} "${n}/", scalar(@{$revision_ids}), ": Revision
#$rev->{revid} of $commit{title}\n";
import_file_revision(\%commit, ($fetch_from == 1), $n_actual,
\%mediafile);
}
--
1.7.9.5
--
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 v4 12/31] git-remote-mediawiki: Change style in a regexp
Change '[\n]' to '\n': brackets are useless here.
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 267c164..86229a1 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -1271,7 +1271,7 @@ sub get_mw_namespace_id {
# Look at configuration file, if the record for that namespace
is
# already cached. Namespaces are stored in form:
# "Name_of_namespace:Id_namespace", ex.: "File:6".
- my @temp = split(/[\n]/, run_git("config --get-all remote."
+ my @temp = split(/\n/, run_git("config --get-all remote."
. $remotename
.".namespaceCache"));
chomp(@temp);
foreach my $ns (@temp) {
--
1.7.9.5
--
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 v4 30/31] git-remote-mediawiki: add a perlcritic rule in Makefile
Option "-2" launches perlcritic with level 2. Levels go from 5 (most pertinent) to 1. Rules of level 1 are mostly a question of style, and are therefore ignored. Signed-off-by: Célestin Matte Signed-off-by: Matthieu Moy --- contrib/mw-to-git/Makefile |5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/contrib/mw-to-git/Makefile b/contrib/mw-to-git/Makefile index 8976aa3..e08b19f 100644 --- a/contrib/mw-to-git/Makefile +++ b/contrib/mw-to-git/Makefile @@ -18,4 +18,7 @@ build install clean: $(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL=$(SCRIPT_PERL_FULL) \ $@-perl-script $(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL=$(SCRIPT_PERL_PREVIEW_FULL) \ -$@-perl-script \ No newline at end of file +$@-perl-script + +perlcritic: + perlcritic -2 *.perl \ No newline at end of file -- 1.7.9.5 -- 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 v3 2/4] prio-queue: priority queue of pointers to structs
Traditionally we used a singly linked list of commits to hold a set
of in-flight commits while traversing history. The most typical use
of the list is to add commits that are newly discovered to it, keep
the list sorted by commit timestamp, pick up the newest one from the
list, and keep digging. The cost of keeping the singly linked list
sorted is nontrivial, and this typical use pattern better matches a
priority queue.
Introduce a prio-queue structure, that can be used either as a LIFO
stack, or a priority queue. This will be used in the next patch to
hold in-flight commits during sort-in-topological-order.
Tests and the idea to make it usable for any "void *" pointers to
"things" are by Jeff King. Bugs are mine.
Signed-off-by: Junio C Hamano
---
.gitignore| 1 +
Makefile | 3 +++
prio-queue.c | 71 +++
prio-queue.h | 45
t/t0009-prio-queue.sh | 50
test-prio-queue.c | 39
6 files changed, 209 insertions(+)
create mode 100644 prio-queue.c
create mode 100644 prio-queue.h
create mode 100755 t/t0009-prio-queue.sh
create mode 100644 test-prio-queue.c
diff --git a/.gitignore b/.gitignore
index 6669bf0..b753817 100644
--- a/.gitignore
+++ b/.gitignore
@@ -190,6 +190,7 @@
/test-mktemp
/test-parse-options
/test-path-utils
+/test-prio-queue
/test-regex
/test-revision-walking
/test-run-command
diff --git a/Makefile b/Makefile
index 598d631..0246194 100644
--- a/Makefile
+++ b/Makefile
@@ -552,6 +552,7 @@ TEST_PROGRAMS_NEED_X += test-mergesort
TEST_PROGRAMS_NEED_X += test-mktemp
TEST_PROGRAMS_NEED_X += test-parse-options
TEST_PROGRAMS_NEED_X += test-path-utils
+TEST_PROGRAMS_NEED_X += test-prio-queue
TEST_PROGRAMS_NEED_X += test-regex
TEST_PROGRAMS_NEED_X += test-revision-walking
TEST_PROGRAMS_NEED_X += test-run-command
@@ -685,6 +686,7 @@ LIB_H += parse-options.h
LIB_H += patch-ids.h
LIB_H += pathspec.h
LIB_H += pkt-line.h
+LIB_H += prio-queue.h
LIB_H += progress.h
LIB_H += prompt.h
LIB_H += quote.h
@@ -824,6 +826,7 @@ LIB_OBJS += pathspec.o
LIB_OBJS += pkt-line.o
LIB_OBJS += preload-index.o
LIB_OBJS += pretty.o
+LIB_OBJS += prio-queue.o
LIB_OBJS += progress.o
LIB_OBJS += prompt.o
LIB_OBJS += quote.o
diff --git a/prio-queue.c b/prio-queue.c
new file mode 100644
index 000..f2a4973
--- /dev/null
+++ b/prio-queue.c
@@ -0,0 +1,71 @@
+#include "cache.h"
+#include "commit.h"
+#include "prio-queue.h"
+
+void clear_prio_queue(struct prio_queue *queue)
+{
+ free(queue->array);
+ queue->nr = 0;
+ queue->alloc = 0;
+ queue->array = NULL;
+}
+
+void prio_queue_put(struct prio_queue *queue, void *thing)
+{
+ prio_queue_compare_fn compare = queue->compare;
+ int ix, parent;
+
+ /* Append at the end */
+ ALLOC_GROW(queue->array, queue->nr + 1, queue->alloc);
+ queue->array[queue->nr++] = thing;
+ if (!compare)
+ return; /* LIFO */
+
+ /* Bubble up the new one */
+ for (ix = queue->nr - 1; ix; ix = parent) {
+ parent = (ix - 1) / 2;
+ if (compare(queue->array[parent], queue->array[ix],
+ queue->cb_data) <= 0)
+ break;
+
+ thing = queue->array[parent];
+ queue->array[parent] = queue->array[ix];
+ queue->array[ix] = thing;
+ }
+}
+
+void *prio_queue_get(struct prio_queue *queue)
+{
+ void *result, *swap;
+ int ix, child;
+ prio_queue_compare_fn compare = queue->compare;
+
+ if (!queue->nr)
+ return NULL;
+ if (!compare)
+ return queue->array[--queue->nr]; /* LIFO */
+
+ result = queue->array[0];
+ if (!--queue->nr)
+ return result;
+
+ queue->array[0] = queue->array[queue->nr];
+
+ /* Push down the one at the root */
+ for (ix = 0; ix * 2 + 1 < queue->nr; ix = child) {
+ child = ix * 2 + 1; /* left */
+ if ((child + 1 < queue->nr) &&
+ (compare(queue->array[child], queue->array[child + 1],
+queue->cb_data) >= 0))
+ child++; /* use right child */
+
+ if (compare(queue->array[ix], queue->array[child],
+ queue->cb_data) <= 0)
+ break;
+
+ swap = queue->array[child];
+ queue->array[child] = queue->array[ix];
+ queue->array[ix] = swap;
+ }
+ return result;
+}
diff --git a/prio-queue.h b/prio-queue.h
new file mode 100644
index 000..ed354a5
--- /dev/null
+++ b/prio-queue.h
@@ -0,0 +1,45 @@
+#ifndef PRIO_QUEUE_H
+#define PRIO_QUEUE_H
+
+/*
+ * A priority queue implementation, primarily for keeping track of
+ * commits in the 'date-order' so that we process them from new to old
+ * as they
[PATCH v4 27/31] git-remote-mediawiki: Fix a typo ("mediwiki" instead of "mediawiki")
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 89b2120..6b6adf2 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -1094,14 +1094,14 @@ sub mw_push_file {
# edit conflicts, considered as non-fast-forward
print {*STDERR} 'Warning: Error ' .
$mediawiki->{error}->{code} .
- ' from mediwiki: ' .
$mediawiki->{error}->{details} .
+ ' from mediawiki: ' .
$mediawiki->{error}->{details} .
".\n";
return ($oldrevid, 'non-fast-forward');
} else {
# Other errors. Shouldn't happen => just die()
die 'Fatal: Error ' .
$mediawiki->{error}->{code} .
- ' from mediwiki: ' .
$mediawiki->{error}->{details} . "\n";
+ ' from mediawiki: ' .
$mediawiki->{error}->{details} . "\n";
}
}
$newrevid = $result->{edit}->{newrevid};
--
1.7.9.5
--
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 v4 24/31] git-remote-mediawiki: Replace "unless" statements with negated "if" statements
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 5174080..4764be5 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -86,11 +86,11 @@ $shallow_import = ($shallow_import eq 'true');
# - by_rev: perform one query per new revision on the remote wiki
# - by_page: query each tracked page for new revision
my $fetch_strategy = run_git("config --get
remote.${remotename}.fetchStrategy");
-unless ($fetch_strategy) {
+if (!$fetch_strategy) {
$fetch_strategy = run_git('config --get mediawiki.fetchStrategy');
}
chomp($fetch_strategy);
-unless ($fetch_strategy) {
+if (!$fetch_strategy) {
$fetch_strategy = 'by_page';
}
@@ -112,7 +112,7 @@ my %basetimestamps;
# deterministic, this means everybody gets the same sha1 for each
# MediaWiki revision.
my $dumb_push = run_git("config --get --bool remote.${remotename}.dumbPush");
-unless ($dumb_push) {
+if (!$dumb_push) {
$dumb_push = run_git('config --get --bool mediawiki.dumbPush');
}
chomp($dumb_push);
@@ -671,7 +671,7 @@ sub fetch_mw_revisions_for_page {
push(@page_revs, $page_rev_ids);
$revnum++;
}
- last unless $result->{'query-continue'};
+ last if (!$result->{'query-continue'});
$query->{rvstartid} =
$result->{'query-continue'}->{revisions}->{rvstartid};
}
if ($shallow_import && @page_revs) {
@@ -1243,7 +1243,7 @@ sub mw_push_revision {
die("Unknown error from mw_push_file()\n");
}
}
- unless ($dumb_push) {
+ if (!$dumb_push) {
run_git(qq(notes --ref=${remotename}/mediawiki add -f
-m "mediawiki_revision: ${mw_revision}" ${sha1_commit}));
run_git(qq(update-ref -m "Git-MediaWiki push"
refs/mediawiki/${remotename}/master ${sha1_commit} ${sha1_child}));
}
@@ -1324,7 +1324,7 @@ sub get_mw_namespace_id {
my $ns = $namespace_id{$name};
my $id;
- unless (defined $ns) {
+ if (!defined $ns) {
print {*STDERR} "No such namespace ${name} on MediaWiki.\n";
$ns = {is_namespace => 0};
$namespace_id{$name} = $ns;
--
1.7.9.5
--
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 v4 29/31] git-remote-mediawiki: Add a .perlcriticrc file
Such a file allows to configure perlcritic. Here, it is used to prevent to remove many unwanted rules and configure one to remove unwanted warnings. Signed-off-by: Célestin Matte Signed-off-by: Matthieu Moy --- contrib/mw-to-git/.perlcriticrc | 28 1 file changed, 28 insertions(+) create mode 100644 contrib/mw-to-git/.perlcriticrc diff --git a/contrib/mw-to-git/.perlcriticrc b/contrib/mw-to-git/.perlcriticrc new file mode 100644 index 000..a1f8451 --- /dev/null +++ b/contrib/mw-to-git/.perlcriticrc @@ -0,0 +1,28 @@ +# These 3 rules demand to add the s, m and x flag to *every* regexp. This is +# overkill and would be harmful for readability. +[-RegularExpressions::RequireExtendedFormatting] +[-RegularExpressions::RequireDotMatchAnything] +[-RegularExpressions::RequireLineBoundaryMatching] + +# This rules says that builtin functions should not be called with parenthesis +# e.g.: (taken from CPAN's documentation) +# open($handle, '>', $filename); #not ok +# open $handle, '>', $filename; #ok +# Applying such a rule would mean modifying a huge number of lines for a +# question of style. +[-CodeLayout::ProhibitParensWithBuiltins] + +# This rule states that each system call should have its return value checked +# The problem is that it includes the print call. Checking every print call's +# return value would be harmful to the code readabilty. +# This configuration keeps all default function but print. +[InputOutput::RequireCheckedSyscalls] +functions = open say close + +# This rules demands to add a dependancy for the Readonly module. This is not +# wished. +[-ValuesAndExpressions::ProhibitConstantPragma] + +# This rule is not really useful (rather a question of style) and produces many +# warnings among the code. +[-ValuesAndExpressions::ProhibitNoisyQuotes] -- 1.7.9.5 -- 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 v4 22/31] git-remote-mediawiki: Modify strings for a better coding-style
- strings which don't need interpolation are single-quoted for more clarity and
slight gain of performance
- interpolation is preferred over concatenation in many cases, for more clarity
- variables are always used with the ${} operator inside strings
- strings including double-quotes are written with qq() so that the quotes do
not have to be escaped
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl | 247 +--
1 file changed, 123 insertions(+), 124 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index da022af..e89bb02 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -18,13 +18,13 @@ use DateTime::Format::ISO8601;
use warnings;
# By default, use UTF-8 to communicate with Git and the user
-binmode STDERR, ":encoding(UTF-8)";
-binmode STDOUT, ":encoding(UTF-8)";
+binmode STDERR, ':encoding(UTF-8)';
+binmode STDOUT, ':encoding(UTF-8)';
use URI::Escape;
# Mediawiki filenames can contain forward slashes. This variable decides by
which pattern they should be replaced
-use constant SLASH_REPLACEMENT => "%2F";
+use constant SLASH_REPLACEMENT => '%2F';
# It's not always possible to delete pages (may require some
# privileges). Deleted pages are replaced with this content.
@@ -35,7 +35,7 @@ use constant DELETED_CONTENT => "[[Category:Deleted]]\n";
use constant EMPTY_CONTENT => "\n";
# used to reflect file creation or deletion in diff.
-use constant NULL_SHA1 => "";
+use constant NULL_SHA1 => '';
# Used on Git's side to reflect empty edit messages on the wiki
use constant EMPTY_MESSAGE => '*Empty MediaWiki Message*';
@@ -49,35 +49,35 @@ my $url = $ARGV[1];
# Accept both space-separated and multiple keys in config file.
# Spaces should be written as _ anyway because we'll use chomp.
-my @tracked_pages = split(/[ \n]/, run_git("config --get-all remote.".
$remotename .".pages"));
+my @tracked_pages = split(/[ \n]/, run_git("config --get-all
remote.${remotename}.pages"));
chomp(@tracked_pages);
# Just like @tracked_pages, but for MediaWiki categories.
-my @tracked_categories = split(/[ \n]/, run_git("config --get-all remote.".
$remotename .".categories"));
+my @tracked_categories = split(/[ \n]/, run_git("config --get-all
remote.${remotename}.categories"));
chomp(@tracked_categories);
# Import media files on pull
-my $import_media = run_git("config --get --bool remote.". $remotename
.".mediaimport");
+my $import_media = run_git("config --get --bool
remote.${remotename}.mediaimport");
chomp($import_media);
-$import_media = ($import_media eq "true");
+$import_media = ($import_media eq 'true');
# Export media files on push
-my $export_media = run_git("config --get --bool remote.". $remotename
.".mediaexport");
+my $export_media = run_git("config --get --bool
remote.${remotename}.mediaexport");
chomp($export_media);
-$export_media = !($export_media eq "false");
+$export_media = !($export_media eq 'false');
-my $wiki_login = run_git("config --get remote.". $remotename .".mwLogin");
+my $wiki_login = run_git("config --get remote.${remotename}.mwLogin");
# Note: mwPassword is discourraged. Use the credential system instead.
-my $wiki_passwd = run_git("config --get remote.". $remotename .".mwPassword");
-my $wiki_domain = run_git("config --get remote.". $remotename .".mwDomain");
+my $wiki_passwd = run_git("config --get remote.${remotename}.mwPassword");
+my $wiki_domain = run_git("config --get remote.${remotename}.mwDomain");
chomp($wiki_login);
chomp($wiki_passwd);
chomp($wiki_domain);
# Import only last revisions (both for clone and fetch)
-my $shallow_import = run_git("config --get --bool remote.". $remotename
.".shallow");
+my $shallow_import = run_git("config --get --bool
remote.${remotename}.shallow");
chomp($shallow_import);
-$shallow_import = ($shallow_import eq "true");
+$shallow_import = ($shallow_import eq 'true');
# Fetch (clone and pull) by revisions instead of by pages. This behavior
# is more efficient when we have a wiki with lots of pages and we fetch
@@ -85,13 +85,13 @@ $shallow_import = ($shallow_import eq "true");
# Possible values:
# - by_rev: perform one query per new revision on the remote wiki
# - by_page: query each tracked page for new revision
-my $fetch_strategy = run_git("config --get remote.$remotename.fetchStrategy");
+my $fetch_strategy = run_git("config --get
remote.${remotename}.fetchStrategy");
unless ($fetch_strategy) {
- $fetch_strategy = run_git("config --get mediawiki.fetchStrategy");
+ $fetch_strategy = run_git('config --get mediawiki.fetchStrategy');
}
chomp($fetch_strategy);
unless ($fetch_strategy) {
- $fetch_strategy = "by_page";
+ $fetch_strategy = 'by_page';
}
# Remember the timestamp correspond
[PATCH v3 1/4] toposort: rename "lifo" field
The primary invariant of sort_in_topological_order() is that a
parent commit is not emitted until all children of it are. When
traversing a forked history like this with "git log C E":
ABC
\
DE
we ensure that A is emitted after all of B, C, D, and E are done, B
has to wait until C is done, and D has to wait until E is done.
In some applications, however, we would further want to control how
these child commits B, C, D and E on two parallel ancestry chains
are shown.
Most of the time, we would want to see C and B emitted together, and
then E and D, and finally A (i.e. the --topo-order output). The
"lifo" parameter of the sort_in_topological_order() function is used
to control this behaviour. We start the traversal by knowing two
commits, C and E. While keeping in mind that we also need to
inspect E later, we pick C first to inspect, and we notice and
record that B needs to be inspected. By structuring the "work to be
done" set as a LIFO stack, we ensure that B is inspected next,
before other in-flight commits we had known that we will need to
inspect, e.g. E.
When showing in --date-order, we would want to see commits ordered
by timestamps, i.e. show C, E, B and D in this order before showing
A, possibly mixing commits from two parallel histories together.
When "lifo" parameter is set to false, the function keeps the "work
to be done" set sorted in the date order to realize this semantics.
After inspecting C, we add B to the "work to be done" set, but the
next commit we inspect from the set is E which is newer than B.
The name "lifo", however, is too strongly tied to the way how the
function implements its behaviour, and does not describe what the
behaviour _means_.
Replace this field with an enum rev_sort_order, with two possible
values: REV_SORT_IN_GRAPH_ORDER and REV_SORT_BY_COMMIT_DATE, and
update the existing code. The mechanical replacement rule is:
"lifo == 0" is equivalent to "sort_order == REV_SORT_BY_COMMIT_DATE"
"lifo == 1" is equivalent to "sort_order == REV_SORT_IN_GRAPH_ORDER"
Signed-off-by: Junio C Hamano
---
builtin/log.c | 2 +-
builtin/show-branch.c | 14 --
commit.c | 12
commit.h | 14 +++---
revision.c| 10 +-
revision.h| 6 +-
6 files changed, 38 insertions(+), 20 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index 8f0b2e8..8d26042 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -205,7 +205,7 @@ static void log_show_early(struct rev_info *revs, struct
commit_list *list)
int i = revs->early_output;
int show_header = 1;
- sort_in_topological_order(&list, revs->lifo);
+ sort_in_topological_order(&list, revs->sort_order);
while (list && i) {
struct commit *commit = list->item;
switch (simplify_commit(revs, commit)) {
diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index d208fd6..7c57985 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -631,7 +631,7 @@ int cmd_show_branch(int ac, const char **av, const char
*prefix)
int num_rev, i, extra = 0;
int all_heads = 0, all_remotes = 0;
int all_mask, all_revs;
- int lifo = 1;
+ enum rev_sort_order sort_order = REV_SORT_IN_GRAPH_ORDER;
char head[128];
const char *head_p;
int head_len;
@@ -666,15 +666,17 @@ int cmd_show_branch(int ac, const char **av, const char
*prefix)
N_("show possible merge bases")),
OPT_BOOLEAN(0, "independent", &independent,
N_("show refs unreachable from any other ref")),
- OPT_BOOLEAN(0, "topo-order", &lifo,
- N_("show commits in topological order")),
+ OPT_SET_INT(0, "topo-order", &sort_order,
+ N_("show commits in topological order"),
+ REV_SORT_IN_GRAPH_ORDER),
OPT_BOOLEAN(0, "topics", &topics,
N_("show only commits not on the first branch")),
OPT_SET_INT(0, "sparse", &dense,
N_("show merges reachable from only one tip"), 0),
- OPT_SET_INT(0, "date-order", &lifo,
+ OPT_SET_INT(0, "date-order", &sort_order,
N_("show commits where no parent comes before its "
- "children"), 0),
+ "children"),
+ REV_SORT_BY_COMMIT_DATE),
{ OPTION_CALLBACK, 'g', "reflog", &reflog_base,
N_("[,]"),
N_("show most recent ref-log entries starting
at "
"base"),
@@ -901,7 +903,7 @@ int cmd_show_branch(int ac, const char **av, const char
*prefix)
exit(0);
/* Sort topologically */
- sort_in_topological_order(&seen, lifo);
+
[PATCH v4 26/31] git-remote-mediawiki: Put non-trivial numeric values in constants.
Non-trivial numeric values (e.g., different from 0, 1 and 2) are placed in
constants at the top of the code to be easily modifiable and to make more sense
Signed-off-by: Célestin Matte
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-remote-mediawiki.perl | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 0bf9a0d..89b2120 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -42,6 +42,16 @@ use constant EMPTY_MESSAGE => '*Empty MediaWiki Message*';
use constant EMPTY => q{};
+# Number of pages taken into account at once in submodule get_mw_page_list
+use constant SLICE_SIZE => 50;
+
+# Number of linked mediafile to get at once in get_linked_mediafiles
+# The query is split in small batches because of the MW API limit of
+# the number of links to be returned (500 links max).
+use constant BATCH_SIZE => 10;
+
+use constant HTTP_CODE_OK => 200;
+
if (@ARGV != 2) {
exit_error_usage();
}
@@ -227,13 +237,13 @@ sub get_mw_page_list {
my $pages = shift;
my @some_pages = @$page_list;
while (@some_pages) {
- my $last_page = 50;
+ my $last_page = SLICE_SIZE;
if ($#some_pages < $last_page) {
$last_page = $#some_pages;
}
my @slice = @some_pages[0..$last_page];
get_mw_first_pages(\@slice, $pages);
- @some_pages = @some_pages[51..$#some_pages];
+ @some_pages = @some_pages[(SLICE_SIZE + 1)..$#some_pages];
}
return;
}
@@ -389,9 +399,7 @@ sub get_linked_mediafiles {
my $pages = shift;
my @titles = map { $_->{title} } values(%{$pages});
- # The query is split in small batches because of the MW API limit of
- # the number of links to be returned (500 links max).
- my $batch = 10;
+ my $batch = BATCH_SIZE;
while (@titles) {
if ($#titles < $batch) {
$batch = $#titles;
@@ -473,7 +481,7 @@ sub download_mw_mediafile {
my $download_url = shift;
my $response = $mediawiki->{ua}->get($download_url);
- if ($response->code == 200) {
+ if ($response->code == HTTP_CODE_OK) {
return $response->decoded_content;
} else {
print {*STDERR} "Error downloading mediafile from :\n";
--
1.7.9.5
--
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 v4 00/31] git-remote-mediawiki: Follow perlcritic's recommandations
New (and hopefully last version) of my series of patches to follow perlcritic's
recommandations
Changes with v3:
- Remove whitespace in [18/28]
- Typo in [09/28]
- Better line split in [22/28]
- A part of the file @@ -610,9 +610,9 @@ had escaped patches [22/31] and
[23/31] for some reason. This is fixed.
- patch [29/31] and [30/31] are new: they add a .perlcriticrc file to ignore
some rules and add a rule in the Makefile for perlcritic
- patch [31/31] is also a new one, which intends to make some error messages
more precise. It comes from an advice from es in the reviewing of v1, that I
had forgotten to add in earlier versions. It is not related to perlcritic, but I
hope it can be included into this series of patches anyway.
Changes with v2:
- Remove patch [02/22] about using the Readonly module
- Split commit [07/22] into 5 different ones
- Split commit [14/22] into 2 different ones
- Patch [17/22] was *not* split: tell me if it is necessary
- Remove wrong change in patch [22/22]
Changes with v1:
- split first commit into 6 different commits
- remove commit [17/18] about moving open() call
- took every other comment into account
Célestin Matte (31):
git-remote-mediawiki: Make a regexp clearer
git-remote-mediawiki: Move "use warnings;" before any instruction
git-remote-mediawiki: Replace :utf8 by :encoding(UTF-8)
git-remote-mediawiki: Always end a subroutine with a return
git-remote-mediawiki: Move a variable declaration at the top of the
code
git-remote-mediawiki: Change syntax of map calls
git-remote-mediawiki: Rewrite unclear line of instructions
git-remote-mediawiki: Remove useless regexp modifier (m)
git-remote-mediawiki: Change the behaviour of a split
git-remote-mediawiki: Change separator of some regexps
git-remote-mediawiki: Change style in a regexp
git-remote-mediawiki: Change style in a regexp
git-remote-mediawiki: Add newline in the end of die() error messages
git-remote-mediawiki: Change the name of a variable
git-remote-mediawiki: Turn double-negated expressions into simple
expressions
git-remote-mediawiki: Remove unused variable $entry
git-remote-mediawiki: Rename a variable ($last) which has the name of
a keyword
git-remote-mediawiki: Assign a variable as undef and make proper
indentation
git-remote-mediawiki: Check return value of open
git-remote-mediawiki: remove import of unused open2
git-remote-mediawiki: Put long code into a subroutine
git-remote-mediawiki: Modify strings for a better coding-style
git-remote-mediawiki: Brace file handles for print for more clarity
git-remote-mediawiki: Replace "unless" statements with negated "if"
statements
git-remote-mediawiki: Don't use quotes for empty strings
git-remote-mediawiki: Put non-trivial numeric values in constants.
git-remote-mediawiki: Fix a typo ("mediwiki" instead of "mediawiki")
git-remote-mediawiki: Clearly rewrite double dereference
git-remote-mediawiki: Add a .perlcriticrc file
git-remote-mediawiki: add a perlcritic rule in Makefile
git-remote-mediawiki: Make error message more precise
contrib/mw-to-git/.perlcriticrc | 28 ++
contrib/mw-to-git/Makefile |5 +-
contrib/mw-to-git/git-remote-mediawiki.perl | 543 +++
3 files changed, 327 insertions(+), 249 deletions(-)
create mode 100644 contrib/mw-to-git/.perlcriticrc
--
1.7.9.5
--
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 v3 07/28] git-remote-mediawiki: Rewrite unclear line of instructions
Célestin Matte writes: > Le 11/06/2013 20:09, Junio C Hamano a écrit : >> Matthieu Moy writes: >> my ($namespace) = @_; my $namespace = shift; My impression has been that both are equally common, >>> >>> The second is the most common in git-remote-mediawiki (but I don't have >>> any preference nor know what is recommended elsewhere). >> >> I wasn't implying I prefer the former. I was just being curious, >> and if the latter is more prevalent in the code _and_ Perlcritique >> does not have any issue with it, it is perfectly fine. > > Perlcritic doesn't have an issue with any of both cases. OK. As this topic is about matching the opinion of Perlcritique, I think it is fine either way (which was the primary thing that I wanted to find out). > I think the second method is clearer when there is only one argument, > because it makes it clear that there is only one. Hmm, from the maintenance point of view, the second one invites the next person to extend this function like this: my $namespace = shift; + my $newargument = shift; + my $anotherargument = shift; If your original were in the first style, instead you would likely to get this: - my ($namespace) = @_; + my ($namespace, $newargument, $anotherargument) = @_; When there is only one argument, it is clear that there is only one argument in either style. It is not a strong factor to pick one style over the other. Once you start taking more than one argument, however, I think "it makes it clear what arguments the function takes" would actually favor the style to split @_ into a list of local variables. But as I said earlier, this patch is about following Perlcritique's advice, and because it does not have opinion on these styles, it is outside the scope of this patch. Thanks for checking. -- 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 11/12] for_each_ref: load all loose refs before packed refs
From: Jeff King
If we are iterating through the refs using for_each_ref (or
any of its sister functions), we can get into a race
condition with a simultaneous "pack-refs --prune" that looks
like this:
0. We have a large number of loose refs, and a few packed
refs. refs/heads/z/foo is loose, with no matching entry
in the packed-refs file.
1. Process A starts iterating through the refs. It loads
the packed-refs file from disk, then starts lazily
traversing through the loose ref directories.
2. Process B, running "pack-refs --prune", writes out the
new packed-refs file. It then deletes the newly packed
refs, including refs/heads/z/foo.
3. Meanwhile, process A has finally gotten to
refs/heads/z (it traverses alphabetically). It
descends, but finds nothing there. It checks its
cached view of the packed-refs file, but it does not
mention anything in "refs/heads/z/" at all (it predates
the new file written by B in step 2).
The traversal completes successfully without mentioning
refs/heads/z/foo at all (the name, of course, isn't
important; but the more refs you have and the farther down
the alphabetical list a ref is, the more likely it is to hit
the race). If refs/heads/z/foo did exist in the packed refs
file at state 0, we would see an entry for it, but it would
show whatever sha1 the ref had the last time it was packed
(which could be an arbitrarily long time ago).
This can be especially dangerous when process A is "git
prune", as it means our set of reachable tips will be
incomplete, and we may erroneously prune objects reachable
from that tip (the same thing can happen if "repack -ad" is
used, as it simply drops unreachable objects that are
packed).
This patch solves it by loading all of the loose refs for
our traversal into our in-memory cache, and then refreshing
the packed-refs cache. Because a pack-refs writer will
always put the new packed-refs file into place before
starting the prune, we know that any loose refs we fail to
see will either truly be missing, or will have already been
put in the packed-refs file by the time we refresh.
Signed-off-by: Michael Haggerty
---
Ditto.
refs.c | 39 +++
1 file changed, 35 insertions(+), 4 deletions(-)
diff --git a/refs.c b/refs.c
index 64f72ab..aa4641b 100644
--- a/refs.c
+++ b/refs.c
@@ -746,6 +746,21 @@ static int do_for_each_entry_in_dirs(struct ref_dir *dir1,
}
/*
+ * Load all of the refs from the dir into our in-memory cache. The hard work
+ * of loading loose refs is done by get_ref_dir(), so we just need to recurse
+ * through all of the sub-directories. We do not even need to care about
+ * sorting, as traversal order does not matter to us.
+ */
+static void prime_ref_dir(struct ref_dir *dir)
+{
+ int i;
+ for (i = 0; i < dir->nr; i++) {
+ struct ref_entry *entry = dir->entries[i];
+ if (entry->flag & REF_DIR)
+ prime_ref_dir(get_ref_dir(entry));
+ }
+}
+/*
* Return true iff refname1 and refname2 conflict with each other.
* Two reference names conflict if one of them exactly matches the
* leading components of the other; e.g., "foo/bar" conflicts with
@@ -1600,15 +1615,31 @@ void warn_dangling_symref(FILE *fp, const char
*msg_fmt, const char *refname)
static int do_for_each_entry(struct ref_cache *refs, const char *base,
each_ref_entry_fn fn, void *cb_data)
{
- struct packed_ref_cache *packed_ref_cache = get_packed_ref_cache(refs);
- struct ref_dir *packed_dir = get_packed_ref_dir(packed_ref_cache);
- struct ref_dir *loose_dir = get_loose_refs(refs);
+ struct packed_ref_cache *packed_ref_cache;
+ struct ref_dir *loose_dir;
+ struct ref_dir *packed_dir;
int retval = 0;
+ /*
+* We must make sure that all loose refs are read before accessing the
+* packed-refs file; this avoids a race condition in which loose refs
+* are migrated to the packed-refs file by a simultaneous process, but
+* our in-memory view is from before the migration.
get_packed_ref_cache()
+* takes care of making sure our view is up to date with what is on
+* disk.
+*/
+ loose_dir = get_loose_refs(refs);
+ if (base && *base) {
+ loose_dir = find_containing_dir(loose_dir, base, 0);
+ }
+ if (loose_dir)
+ prime_ref_dir(loose_dir);
+
+ packed_ref_cache = get_packed_ref_cache(refs);
acquire_packed_ref_cache(packed_ref_cache);
+ packed_dir = get_packed_ref_dir(packed_ref_cache);
if (base && *base) {
packed_dir = find_containing_dir(packed_dir, base, 0);
- loose_dir = find_containing_dir(loose_dir, base, 0);
}
if (packed_dir && loose_dir) {
--
1.8.3
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to
[PATCH 07/12] packed_ref_cache: increment refcount when locked
Increment the packed_ref_cache reference count while it is locked to
prevent its being freed.
Signed-off-by: Michael Haggerty
---
refs.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/refs.c b/refs.c
index d8e8ce2..92c8e97 100644
--- a/refs.c
+++ b/refs.c
@@ -816,7 +816,9 @@ struct packed_ref_cache {
/*
* Iff the packed-refs file associated with this instance is
* currently locked for writing, this points at the associated
-* lock (which is owned by somebody else).
+* lock (which is owned by somebody else). The referrer count
+* is also incremented when the file is locked and decremented
+* when it is unlocked.
*/
struct lock_file *lock;
@@ -2099,6 +2101,8 @@ int lock_packed_refs(struct lock_file *lock, int flags)
packed_ref_cache = get_packed_ref_cache(&ref_cache);
packed_ref_cache->lock = lock;
packed_ref_cache->fd = fd;
+ /* Increment the reference count to prevent it from being freed: */
+ acquire_packed_ref_cache(packed_ref_cache);
return 0;
}
@@ -2119,6 +2123,7 @@ int commit_packed_refs(void)
error = -1;
packed_ref_cache->lock = NULL;
packed_ref_cache->fd = -1;
+ release_packed_ref_cache(packed_ref_cache);
return error;
}
@@ -2132,6 +2137,7 @@ void rollback_packed_refs(void)
rollback_lock_file(packed_ref_cache->lock);
packed_ref_cache->lock = NULL;
packed_ref_cache->fd = -1;
+ release_packed_ref_cache(packed_ref_cache);
clear_packed_ref_cache(&ref_cache);
}
--
1.8.3
--
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 04/12] refs: implement simple transactions for the packed-refs file
Handle simple transactions for the packed-refs file at the
packed_ref_cache level via new functions lock_packed_refs(),
commit_packed_refs(), and rollback_packed_refs().
Only allow the packed ref cache to be modified (via add_packed_ref())
while the packed refs file is locked.
Change clone to add the new references within a transaction.
Signed-off-by: Michael Haggerty
---
The API docs are not clear about whether it is kosher to read
lock_file::fd directly. It is only done in one file outside of
lockfile.c. So this patch stores the fd of the lockfile separately in
struct packed_ref_cache, even though the same struct also has a
pointer to the struct lock_file.
So please let me know if it is OK to read lock_file::fd directly. If
so, then I will drop the fd member of struct packed_ref_cache, as well
as the local variable "fd" in lock_packed_refs().
builtin/clone.c | 7 -
refs.c | 91 +++--
refs.h | 27 +++--
3 files changed, 106 insertions(+), 19 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 66bff57..b0c000a 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -489,17 +489,22 @@ static struct ref *wanted_peer_refs(const struct ref
*refs,
return local_refs;
}
+static struct lock_file packed_refs_lock;
+
static void write_remote_refs(const struct ref *local_refs)
{
const struct ref *r;
+ lock_packed_refs(&packed_refs_lock, LOCK_DIE_ON_ERROR);
+
for (r = local_refs; r; r = r->next) {
if (!r->peer_ref)
continue;
add_packed_ref(r->peer_ref->name, r->old_sha1);
}
- pack_refs(PACK_REFS_ALL);
+ if (commit_packed_refs())
+ die_errno("unable to overwrite old ref-pack file");
}
static void write_followtags(const struct ref *refs, const char *msg)
diff --git a/refs.c b/refs.c
index 4cee36b..114bd5b 100644
--- a/refs.c
+++ b/refs.c
@@ -804,6 +804,16 @@ static int is_refname_available(const char *refname, const
char *oldrefname,
struct packed_ref_cache {
struct ref_entry *root;
+
+ /*
+* Iff the packed-refs file associated with this instance is
+* currently locked for writing, this points at the associated
+* lock (which is owned by somebody else).
+*/
+ struct lock_file *lock;
+
+ /* If locked, the file descriptor of the lock file. */
+ int fd;
};
/*
@@ -825,6 +835,8 @@ static struct ref_cache {
static void clear_packed_ref_cache(struct ref_cache *refs)
{
if (refs->packed) {
+ if (refs->packed->lock)
+ die("internal error: packed-ref cache cleared while
locked");
free_ref_entry(refs->packed->root);
free(refs->packed);
refs->packed = NULL;
@@ -1009,6 +1021,7 @@ static struct packed_ref_cache
*get_packed_ref_cache(struct ref_cache *refs)
refs->packed = xcalloc(1, sizeof(*refs->packed));
refs->packed->root = create_dir_entry(refs, "", 0, 0);
+ refs->packed->fd = -1;
if (*refs->name)
packed_refs_file = git_path_submodule(refs->name,
"packed-refs");
else
@@ -1034,7 +1047,12 @@ static struct ref_dir *get_packed_refs(struct ref_cache
*refs)
void add_packed_ref(const char *refname, const unsigned char *sha1)
{
- add_ref(get_packed_refs(&ref_cache),
+ struct packed_ref_cache *packed_ref_cache =
+ get_packed_ref_cache(&ref_cache);
+
+ if (!packed_ref_cache->lock)
+ die("internal error: packed refs not locked");
+ add_ref(get_packed_ref_dir(packed_ref_cache),
create_ref_entry(refname, sha1, REF_ISPACKED, 1));
}
@@ -2031,6 +2049,56 @@ static int write_packed_entry_fn(struct ref_entry
*entry, void *cb_data)
return 0;
}
+int lock_packed_refs(struct lock_file *lock, int flags)
+{
+ int fd;
+ struct packed_ref_cache *packed_ref_cache;
+
+ /* Discard the old cache because it might be invalid: */
+ clear_packed_ref_cache(&ref_cache);
+ fd = hold_lock_file_for_update(lock, git_path("packed-refs"), flags);
+ if (fd < 0)
+ return -1;
+ /* Read the current packed-refs while holding the lock: */
+ packed_ref_cache = get_packed_ref_cache(&ref_cache);
+ packed_ref_cache->lock = lock;
+ packed_ref_cache->fd = fd;
+ return 0;
+}
+
+int commit_packed_refs(void)
+{
+ struct packed_ref_cache *packed_ref_cache =
+ get_packed_ref_cache(&ref_cache);
+ int fd = packed_ref_cache->fd;
+ int error = 0;
+
+ if (!packed_ref_cache->lock)
+ die("internal error: packed-refs not locked");
+ write_or_die(fd, PACKED_REFS_HEADER, strlen(PACKED_REFS_HEADER));
+
+ do_for_each_entry_in_dir(get_packed_ref_dir(packed_ref_cache),
[PATCH 09/12] add a stat_validity struct
It can sometimes be useful to know whether a path in the
filesystem has been updated without going to the work of
opening and re-reading its content. We trust the stat()
information on disk already to handle index updates, and we
can use the same trick here.
This patch introduces a "stat_validity" struct which
encapsulates the concept of checking the stat-freshness of a
file. It is implemented on top of "struct stat_data" to
reuse the logic about which stat entries to trust for a
particular platform, but hides the complexity behind two
simple functions: check and update.
Signed-off-by: Michael Haggerty
---
This is *very* similar to a patch by Jeff King [1],
except that it is based on the struct stat_data that I extracted from
cache_entry rather than using cache_entries directly. I would have
left Peff the author except that I don't want to risk putting him on
the hook for any mistakes that I might have made. But if it is
appropriate, don't hesitate to make him author again.
[1] http://article.gmane.org/gmane.comp.version-control.git/223526
cache.h | 27 +++
read-cache.c | 30 ++
2 files changed, 57 insertions(+)
diff --git a/cache.h b/cache.h
index 207f849..15f5110 100644
--- a/cache.h
+++ b/cache.h
@@ -1358,4 +1358,31 @@ int checkout_fast_forward(const unsigned char *from,
int sane_execvp(const char *file, char *const argv[]);
+/*
+ * A struct to encapsulate the concept of whether a file has changed
+ * since we last checked it. This uses criteria similar to those used
+ * for the index.
+ */
+struct stat_validity {
+ struct stat_data *sd;
+};
+
+void stat_validity_clear(struct stat_validity *sv);
+
+/*
+ * Returns 1 if the path is a regular file (or a symlink to a regular
+ * file) and matches the saved stat_validity, 0 otherwise. A missing
+ * or inaccessible file is considered a match if the struct was just
+ * initialized, or if the previous update found an inaccessible file.
+ */
+int stat_validity_check(struct stat_validity *sv, const char *path);
+
+/*
+ * Update the stat_validity from a file opened at descriptor fd. If
+ * the file is missing, inaccessible, or not a regular file, then
+ * future calls to stat_validity_check will match iff one of those
+ * conditions continues to be true.
+ */
+void stat_validity_update(struct stat_validity *sv, int fd);
+
#endif /* CACHE_H */
diff --git a/read-cache.c b/read-cache.c
index 4c4328e..73e85a4 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1948,3 +1948,33 @@ void *read_blob_data_from_index(struct index_state
*istate, const char *path, un
*size = sz;
return data;
}
+
+void stat_validity_clear(struct stat_validity *sv)
+{
+ free(sv->sd);
+ sv->sd = NULL;
+}
+
+int stat_validity_check(struct stat_validity *sv, const char *path)
+{
+ struct stat st;
+
+ if (stat(path, &st) < 0)
+ return sv->sd == NULL;
+ if (!sv->sd)
+ return 0;
+ return S_ISREG(st.st_mode) && !match_stat_data(sv->sd, &st);
+}
+
+void stat_validity_update(struct stat_validity *sv, int fd)
+{
+ struct stat st;
+
+ if (fstat(fd, &st) < 0 || !S_ISREG(st.st_mode))
+ stat_validity_clear(sv);
+ else {
+ if (!sv->sd)
+ sv->sd = xcalloc(1, sizeof(struct stat_data));
+ fill_stat_data(sv->sd, &st);
+ }
+}
--
1.8.3
--
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 06/12] do_for_each_entry(): increment the packed refs cache refcount
This function calls a user-supplied callback function which could do
something that causes the packed refs cache to be invalidated. So
acquire a reference count on the data structure to prevent our copy
from being freed while we are iterating over it.
Signed-off-by: Michael Haggerty
---
refs.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/refs.c b/refs.c
index b0d77a7..d8e8ce2 100644
--- a/refs.c
+++ b/refs.c
@@ -1587,10 +1587,12 @@ void warn_dangling_symref(FILE *fp, const char
*msg_fmt, const char *refname)
static int do_for_each_entry(struct ref_cache *refs, const char *base,
each_ref_entry_fn fn, void *cb_data)
{
- struct ref_dir *packed_dir = get_packed_refs(refs);
+ struct packed_ref_cache *packed_ref_cache = get_packed_ref_cache(refs);
+ struct ref_dir *packed_dir = get_packed_ref_dir(packed_ref_cache);
struct ref_dir *loose_dir = get_loose_refs(refs);
int retval = 0;
+ acquire_packed_ref_cache(packed_ref_cache);
if (base && *base) {
packed_dir = find_containing_dir(packed_dir, base, 0);
loose_dir = find_containing_dir(loose_dir, base, 0);
@@ -1611,6 +1613,7 @@ static int do_for_each_entry(struct ref_cache *refs,
const char *base,
loose_dir, 0, fn, cb_data);
}
+ release_packed_ref_cache(packed_ref_cache);
return retval;
}
--
1.8.3
--
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 02/12] pack_refs(): split creation of packed refs and entry writing
Split pack_refs() into multiple passes:
* Iterate over loose refs. For each one that can be turned into a
packed ref, create a corresponding entry in the packed refs cache.
* Write the packed refs to the packed-refs file.
This change isolates the mutation of the packed-refs file to a single
place.
Signed-off-by: Michael Haggerty
---
refs.c | 48 ++--
1 file changed, 34 insertions(+), 14 deletions(-)
diff --git a/refs.c b/refs.c
index dccfe14..bb3640f 100644
--- a/refs.c
+++ b/refs.c
@@ -2019,35 +2019,50 @@ struct ref_to_prune {
struct pack_refs_cb_data {
unsigned int flags;
+ struct ref_dir *packed_refs;
struct ref_to_prune *ref_to_prune;
- int fd;
};
-static int pack_one_ref(struct ref_entry *entry, void *cb_data)
+/*
+ * An each_ref_entry_fn that is run over loose references only. If
+ * the loose reference can be packed, add an entry in the packed ref
+ * cache. If the reference should be pruned, also add it to
+ * ref_to_prune in the pack_refs_cb_data.
+ */
+static int pack_if_possible_fn(struct ref_entry *entry, void *cb_data)
{
struct pack_refs_cb_data *cb = cb_data;
enum peel_status peel_status;
+ struct ref_entry *packed_entry;
int is_tag_ref = !prefixcmp(entry->name, "refs/tags/");
- /* ALWAYS pack refs that were already packed or are tags */
- if (!(cb->flags & PACK_REFS_ALL) && !is_tag_ref &&
- !(entry->flag & REF_ISPACKED))
+ /* ALWAYS pack tags */
+ if (!(cb->flags & PACK_REFS_ALL) && !is_tag_ref)
return 0;
/* Do not pack symbolic or broken refs: */
if ((entry->flag & REF_ISSYMREF) || !ref_resolves_to_object(entry))
return 0;
+ /* Add a packed ref cache entry equivalent to the loose entry. */
peel_status = peel_entry(entry, 1);
if (peel_status != PEEL_PEELED && peel_status != PEEL_NON_TAG)
die("internal error peeling reference %s (%s)",
entry->name, sha1_to_hex(entry->u.value.sha1));
- write_packed_entry(cb->fd, entry->name, entry->u.value.sha1,
- peel_status == PEEL_PEELED ?
- entry->u.value.peeled : NULL);
+ packed_entry = find_ref(cb->packed_refs, entry->name);
+ if (packed_entry) {
+ /* Overwrite existing packed entry with info from loose entry */
+ packed_entry->flag = REF_ISPACKED | REF_KNOWS_PEELED;
+ hashcpy(packed_entry->u.value.sha1, entry->u.value.sha1);
+ } else {
+ packed_entry = create_ref_entry(entry->name,
entry->u.value.sha1,
+ REF_ISPACKED |
REF_KNOWS_PEELED, 0);
+ add_ref(cb->packed_refs, packed_entry);
+ }
+ hashcpy(packed_entry->u.value.peeled, entry->u.value.peeled);
- /* If the ref was already packed, there is no need to prune it. */
- if ((cb->flags & PACK_REFS_PRUNE) && !(entry->flag & REF_ISPACKED)) {
+ /* Schedule the loose reference for pruning if requested. */
+ if ((cb->flags & PACK_REFS_PRUNE)) {
int namelen = strlen(entry->name) + 1;
struct ref_to_prune *n = xcalloc(1, sizeof(*n) + namelen);
hashcpy(n->sha1, entry->u.value.sha1);
@@ -2114,16 +2129,21 @@ static struct lock_file packlock;
int pack_refs(unsigned int flags)
{
struct pack_refs_cb_data cbdata;
+ int fd;
memset(&cbdata, 0, sizeof(cbdata));
cbdata.flags = flags;
- cbdata.fd = hold_lock_file_for_update(&packlock,
git_path("packed-refs"),
- LOCK_DIE_ON_ERROR);
+ fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"),
+ LOCK_DIE_ON_ERROR);
+ cbdata.packed_refs = get_packed_refs(&ref_cache);
- write_or_die(cbdata.fd, PACKED_REFS_HEADER, strlen(PACKED_REFS_HEADER));
+ do_for_each_entry_in_dir(get_loose_refs(&ref_cache), 0,
+pack_if_possible_fn, &cbdata);
+
+ write_or_die(fd, PACKED_REFS_HEADER, strlen(PACKED_REFS_HEADER));
+ do_for_each_entry_in_dir(cbdata.packed_refs, 0, write_packed_entry_fn,
&fd);
- do_for_each_entry(&ref_cache, "", pack_one_ref, &cbdata);
if (commit_lock_file(&packlock) < 0)
die_errno("unable to overwrite old ref-pack file");
prune_refs(cbdata.ref_to_prune);
--
1.8.3
--
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 08/12] Extract a struct stat_data from cache_entry
Add public functions fill_stat_data() and match_stat_data() to work
with it. This infrastructure will later be used to check the validity
of other types of file.
Signed-off-by: Michael Haggerty
---
I'm not too familiar with this part of the code, so please make sure
that I've put the dividing line at the right place.
builtin/ls-files.c | 12 +++--
cache.h| 33 +---
read-cache.c | 151 +
3 files changed, 116 insertions(+), 80 deletions(-)
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 2202072..6a0730f 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -165,11 +165,13 @@ static void show_ce_entry(const char *tag, struct
cache_entry *ce)
}
write_name(ce->name, ce_namelen(ce));
if (debug_mode) {
- printf(" ctime: %d:%d\n", ce->ce_ctime.sec, ce->ce_ctime.nsec);
- printf(" mtime: %d:%d\n", ce->ce_mtime.sec, ce->ce_mtime.nsec);
- printf(" dev: %d\tino: %d\n", ce->ce_dev, ce->ce_ino);
- printf(" uid: %d\tgid: %d\n", ce->ce_uid, ce->ce_gid);
- printf(" size: %d\tflags: %x\n", ce->ce_size, ce->ce_flags);
+ struct stat_data *sd = &ce->ce_stat_data;
+
+ printf(" ctime: %d:%d\n", sd->sd_ctime.sec, sd->sd_ctime.nsec);
+ printf(" mtime: %d:%d\n", sd->sd_mtime.sec, sd->sd_mtime.nsec);
+ printf(" dev: %d\tino: %d\n", sd->sd_dev, sd->sd_ino);
+ printf(" uid: %d\tgid: %d\n", sd->sd_uid, sd->sd_gid);
+ printf(" size: %d\tflags: %x\n", sd->sd_size, ce->ce_flags);
}
}
diff --git a/cache.h b/cache.h
index df532f8..207f849 100644
--- a/cache.h
+++ b/cache.h
@@ -119,15 +119,19 @@ struct cache_time {
unsigned int nsec;
};
+struct stat_data {
+ struct cache_time sd_ctime;
+ struct cache_time sd_mtime;
+ unsigned int sd_dev;
+ unsigned int sd_ino;
+ unsigned int sd_uid;
+ unsigned int sd_gid;
+ unsigned int sd_size;
+};
+
struct cache_entry {
- struct cache_time ce_ctime;
- struct cache_time ce_mtime;
- unsigned int ce_dev;
- unsigned int ce_ino;
+ struct stat_data ce_stat_data;
unsigned int ce_mode;
- unsigned int ce_uid;
- unsigned int ce_gid;
- unsigned int ce_size;
unsigned int ce_flags;
unsigned int ce_namelen;
unsigned char sha1[20];
@@ -509,6 +513,21 @@ extern int limit_pathspec_to_literal(void);
#define HASH_FORMAT_CHECK 2
extern int index_fd(unsigned char *sha1, int fd, struct stat *st, enum
object_type type, const char *path, unsigned flags);
extern int index_path(unsigned char *sha1, const char *path, struct stat *st,
unsigned flags);
+
+/*
+ * Record to sd the data from st that we use to check whether a file
+ * might have changed.
+ */
+extern void fill_stat_data(struct stat_data *sd, struct stat *st);
+
+/*
+ * Return 0 if st is consistent with a file not having been changed
+ * since sd was filled. If there are differences, return a
+ * combination of MTIME_CHANGED, CTIME_CHANGED, OWNER_CHANGED,
+ * INODE_CHANGED, and DATA_CHANGED.
+ */
+extern int match_stat_data(struct stat_data *sd, struct stat *st);
+
extern void fill_stat_cache_info(struct cache_entry *ce, struct stat *st);
#define REFRESH_REALLY 0x0001 /* ignore_valid */
diff --git a/read-cache.c b/read-cache.c
index 04ed561..4c4328e 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -67,6 +67,61 @@ void rename_index_entry_at(struct index_state *istate, int
nr, const char *new_n
add_index_entry(istate, new,
ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE);
}
+void fill_stat_data(struct stat_data *sd, struct stat *st)
+{
+ sd->sd_ctime.sec = (unsigned int)st->st_ctime;
+ sd->sd_mtime.sec = (unsigned int)st->st_mtime;
+ sd->sd_ctime.nsec = ST_CTIME_NSEC(*st);
+ sd->sd_mtime.nsec = ST_MTIME_NSEC(*st);
+ sd->sd_dev = st->st_dev;
+ sd->sd_ino = st->st_ino;
+ sd->sd_uid = st->st_uid;
+ sd->sd_gid = st->st_gid;
+ sd->sd_size = st->st_size;
+}
+
+int match_stat_data(struct stat_data *sd, struct stat *st)
+{
+ int changed = 0;
+
+ if (sd->sd_mtime.sec != (unsigned int)st->st_mtime)
+ changed |= MTIME_CHANGED;
+ if (trust_ctime && check_stat &&
+ sd->sd_ctime.sec != (unsigned int)st->st_ctime)
+ changed |= CTIME_CHANGED;
+
+#ifdef USE_NSEC
+ if (check_stat && sd->sd_mtime.nsec != ST_MTIME_NSEC(*st))
+ changed |= MTIME_CHANGED;
+ if (trust_ctime && check_stat &&
+ sd->sd_ctime.nsec != ST_CTIME_NSEC(*st))
+ changed |= CTIME_CHANGED;
+#endif
+
+ if (check_stat) {
+ if (sd->sd_uid != (unsigned int) st->st_uid ||
+ sd->sd_gid != (unsigned int) st->st_gid)
+ changed |= OWNER_CHANGED;
+ if
[PATCH 10/12] get_packed_ref_cache: reload packed-refs file when it changes
From: Jeff King
Once we read the packed-refs file into memory, we cache it
to save work on future ref lookups. However, our cache may
be out of date with respect to what is on disk if another
process is simultaneously packing the refs. Normally it
is acceptable for us to be a little out of date, since there
is no guarantee whether we read the file before or after the
simultaneous update. However, there is an important special
case: our packed-refs file must be up to date with respect
to any loose refs we read. Otherwise, we risk the following
race condition:
0. There exists a loose ref refs/heads/master.
1. Process A starts and looks up the ref "master". It
first checks $GIT_DIR/master, which does not exist. It
then loads (and caches) the packed-refs file to see if
"master" exists in it, which it does not.
2. Meanwhile, process B runs "pack-refs --all --prune". It
creates a new packed-refs file which contains
refs/heads/master, and removes the loose copy at
$GIT_DIR/refs/heads/master.
3. Process A continues its lookup, and eventually tries
$GIT_DIR/refs/heads/master. It sees that the loose ref
is missing, and falls back to the packed-refs file. But
it examines its cached version, which does not have
refs/heads/master. After trying a few other prefixes,
it reports master as a non-existent ref.
There are many variants (e.g., step 1 may involve process A
looking up another ref entirely, so even a fully qualified
refname can fail). One of the most interesting ones is if
"refs/heads/master" is already packed. In that case process
A will not see it as missing, but rather will report
whatever value happened to be in the packed-refs file before
process B repacked (which might be an arbitrarily old
value).
We can fix this by making sure we reload the packed-refs
file from disk after looking at any loose refs. That's
unacceptably slow, so we can check its stat()-validity as a
proxy, and read it only when it appears to have changed.
Reading the packed-refs file after performing any loose-ref
system calls is sufficient because we know the ordering of
the pack-refs process: it always makes sure the newly
written packed-refs file is installed into place before
pruning any loose refs. As long as those operations by B
appear in their executed order to process A, by the time A
sees the missing loose ref, the new packed-refs file must be
in place.
Signed-off-by: Michael Haggerty
---
This is Peff's work, rebased and with some smallish changes to fit it
in with the packed_ref_cache data structure.
refs.c | 21 -
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/refs.c b/refs.c
index 92c8e97..64f72ab 100644
--- a/refs.c
+++ b/refs.c
@@ -824,6 +824,9 @@ struct packed_ref_cache {
/* If locked, the file descriptor of the lock file. */
int fd;
+
+ /* The metadata from when this packed-refs cache was read */
+ struct stat_validity validity;
};
/*
@@ -858,6 +861,7 @@ static int release_packed_ref_cache(struct packed_ref_cache
*packed_refs)
{
if (!--packed_refs->referrers) {
free_ref_entry(packed_refs->root);
+ stat_validity_clear(&packed_refs->validity);
free(packed_refs);
return 1;
} else {
@@ -1049,20 +1053,27 @@ static void read_packed_refs(FILE *f, struct ref_dir
*dir)
*/
static struct packed_ref_cache *get_packed_ref_cache(struct ref_cache *refs)
{
+ const char *packed_refs_file;
+
+ if (*refs->name)
+ packed_refs_file = git_path_submodule(refs->name,
"packed-refs");
+ else
+ packed_refs_file = git_path("packed-refs");
+
+ if (refs->packed &&
+ !stat_validity_check(&refs->packed->validity, packed_refs_file))
+ clear_packed_ref_cache(refs);
+
if (!refs->packed) {
- const char *packed_refs_file;
FILE *f;
refs->packed = xcalloc(1, sizeof(*refs->packed));
acquire_packed_ref_cache(refs->packed);
refs->packed->root = create_dir_entry(refs, "", 0, 0);
refs->packed->fd = -1;
- if (*refs->name)
- packed_refs_file = git_path_submodule(refs->name,
"packed-refs");
- else
- packed_refs_file = git_path("packed-refs");
f = fopen(packed_refs_file, "r");
if (f) {
+ stat_validity_update(&refs->packed->validity,
fileno(f));
read_packed_refs(f, get_ref_dir(refs->packed->root));
fclose(f);
}
--
1.8.3
--
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 12/12] refs: do not invalidate the packed-refs cache unnecessarily
Now that we keep track of the packed-refs file metadata, we can detect
when the packed-refs file has been modified since we last read it, and
we do so automatically every time that get_packed_ref_cache() is
called. So there is no need to invalidate the cache automatically
when lock_packed_refs() is called; usually the old copy will still be
valid.
Signed-off-by: Michael Haggerty
---
This patch is optional. It makes the assumption that the metadata
stored in stat_validity are adequate to reliably detect when the
packed-refs file has changed. Given that we are about to rewrite the
file, it is perhaps even more crucial not to make a mistake in this
codepath than in others. So if the stat_validity check is not
considered safe enough, it might be prudent to omit this patch and
continue to reload the packed-refs data here unconditionally.
refs.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/refs.c b/refs.c
index aa4641b..d9cbfc4 100644
--- a/refs.c
+++ b/refs.c
@@ -2134,12 +2134,15 @@ int lock_packed_refs(struct lock_file *lock, int flags)
int fd;
struct packed_ref_cache *packed_ref_cache;
- /* Discard the old cache because it might be invalid: */
- clear_packed_ref_cache(&ref_cache);
fd = hold_lock_file_for_update(lock, git_path("packed-refs"), flags);
if (fd < 0)
return -1;
- /* Read the current packed-refs while holding the lock: */
+ /*
+* Get the current packed-refs while holding the lock. If the
+* packed-refs file has been modified since we last read it,
+* this will automatically invalidate the cache and re-read
+* the packed-refs file.
+*/
packed_ref_cache = get_packed_ref_cache(&ref_cache);
packed_ref_cache->lock = lock;
packed_ref_cache->fd = fd;
--
1.8.3
--
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 00/12] Fix some reference-related races
*This patch series must be built on top of mh/reflife.*
This patch series fixes some races reading loose and packed refs.
Most of the problems, and some of the solutions, were pointed out by
Jeff King [1] but some other work was necessary to prevent his fixes
from causing problems elsewhere.
The basic race being addressed is that at any time "pack-refs --prune"
might be run at any time. It rewrites the packed-refs file and then
deletes the just-packed loose refs.
Readers, then, to get a self-consistent snapshot of references [2],
must be sure to read all of the loose references it will need *before*
reading the packed references (or at least verifying that the packed
references that it read earlier are still valid). But given the
lazy-loading of the loose references cache, this was not always the
case.
So the core of this patch series is to force loose references for an
iteration to be read all at once into the cache, and *then* to verify
that the packed-refs cache is up-to-date and if not to reload it.
Similarly, when looking up single references, a loose reference is
sought first, and then the validity of the packed-refs cache is
verified, and then the loose reference is sought in the cache.
The problem is that there was a lot of code that assumed that the
lifetime of the reference cache was essentially infinite. The
mh/reflife patch series (which has made it to next) fixed callers who
retained pointers to refnames in the cache.
The other problem was that the for_each_ref() functions will die if
the ref cache that they are iterating over is freed out from under
them. This problem is solved by using reference counts to avoid
freeing the old packed ref cache (even if it is no longer valid) until
all users are done with it.
Once those are done, it is possible to invalidate the packed refs
cache when needed. So (1) we always read all loose references that
will be needed in an iteration before the iteration starts, and (2) we
add a check (based on file metadata) whenever the packed-refs cache is
accessed that it is still up-to-date WRT the packed-refs file, and if
not reread it (but leave the old copy in memory as long as its
refcount is nonzero).
Along the way, this patch series adds simple transactions around the
packed-refs file/cache. The transaction interface is public. I think
this is a step in a good direction, because other race conditions not
addressed by this patch series are likely to require transactions
across the whole reference namespace to be made 100% reliable.
As a stress test, the test suite can be run with a simulated
"hyperactive repository" in which the packed-refs file is made to look
like it changes every time it is checked (except when its lock is
held):
refs.c
@@ -1075,8 +1075,8 @@ static struct packed_ref_cache
*get_packed_ref_cache(struct ref_cache *refs)
else
packed_refs_file = git_path("packed-refs");
- if (refs->packed &&
- !stat_validity_check(&refs->packed->validity, packed_refs_file))
+ if (refs->packed && !refs->packed->lock
+ /*!stat_validity_check(&refs->packed->validity, packed_refs_file)*/)
clear_packed_ref_cache(refs);
if (!refs->packed) {
It passes the stress test.
[1] http://thread.gmane.org/gmane.comp.version-control.git/223299/focus=223526
Jeff King (2):
get_packed_ref_cache: reload packed-refs file when it changes
for_each_ref: load all loose refs before packed refs
Michael Haggerty (10):
repack_without_ref(): split list curation and entry writing
pack_refs(): split creation of packed refs and entry writing
refs: wrap the packed refs cache in a level of indirection
refs: implement simple transactions for the packed-refs file
refs: manage lifetime of packed refs cache via reference counting
do_for_each_entry(): increment the packed refs cache refcount
packed_ref_cache: increment refcount when locked
Extract a struct stat_data from cache_entry
add a stat_validity struct
refs: do not invalidate the packed-refs cache unnecessarily
builtin/clone.c| 7 +-
builtin/ls-files.c | 12 ++-
cache.h| 60 +--
read-cache.c | 181 +++
refs.c | 308 -
refs.h | 27 -
6 files changed, 464 insertions(+), 131 deletions(-)
--
1.8.3
--
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 01/12] repack_without_ref(): split list curation and entry writing
Split repack_without_ref() into multiple passes:
* collect the list of refnames that should be deleted from packed_refs
* delete those refnames from the cache
* write the remainder to the packed-refs file
The purpose of this change is to make the "write the remainder" part
reusable.
Signed-off-by: Michael Haggerty
---
refs.c | 48 +---
1 file changed, 37 insertions(+), 11 deletions(-)
diff --git a/refs.c b/refs.c
index d17931a..dccfe14 100644
--- a/refs.c
+++ b/refs.c
@@ -1994,6 +1994,23 @@ static void write_packed_entry(int fd, char *refname,
unsigned char *sha1,
}
}
+/*
+ * An each_ref_entry_fn that writes the entry to a packed-refs file.
+ */
+static int write_packed_entry_fn(struct ref_entry *entry, void *cb_data)
+{
+ int *fd = cb_data;
+ enum peel_status peel_status = peel_entry(entry, 0);
+
+ if (peel_status != PEEL_PEELED && peel_status != PEEL_NON_TAG)
+ error("internal error: %s is not a valid packed reference!",
+ entry->name);
+ write_packed_entry(*fd, entry->name, entry->u.value.sha1,
+ peel_status == PEEL_PEELED ?
+ entry->u.value.peeled : NULL);
+ return 0;
+}
+
struct ref_to_prune {
struct ref_to_prune *next;
unsigned char sha1[20];
@@ -2113,14 +2130,18 @@ int pack_refs(unsigned int flags)
return 0;
}
-static int repack_ref_fn(struct ref_entry *entry, void *cb_data)
+/*
+ * If entry should be deleted from packed-refs, add it to the string
+ * list pointed to by cb_data.
+ */
+static int curate_packed_ref_fn(struct ref_entry *entry, void *cb_data)
{
- int *fd = cb_data;
- enum peel_status peel_status;
+ struct string_list *refs_to_delete = cb_data;
if (entry->flag & REF_ISBROKEN) {
/* This shouldn't happen to packed refs. */
error("%s is broken!", entry->name);
+ string_list_append(refs_to_delete, entry->name);
return 0;
}
if (!has_sha1_file(entry->u.value.sha1)) {
@@ -2130,7 +2151,7 @@ static int repack_ref_fn(struct ref_entry *entry, void
*cb_data)
if (read_ref_full(entry->name, sha1, 0, &flags))
/* We should at least have found the packed ref. */
die("Internal error");
- if ((flags & REF_ISSYMREF) || !(flags & REF_ISPACKED))
+ if ((flags & REF_ISSYMREF) || !(flags & REF_ISPACKED)) {
/*
* This packed reference is overridden by a
* loose reference, so it is OK that its value
@@ -2139,9 +2160,11 @@ static int repack_ref_fn(struct ref_entry *entry, void
*cb_data)
* collected. For this purpose we don't even
* care whether the loose reference itself is
* invalid, broken, symbolic, etc. Silently
-* omit the packed reference from the output.
+* remove the packed reference.
*/
+ string_list_append(refs_to_delete, entry->name);
return 0;
+ }
/*
* There is no overriding loose reference, so the fact
* that this reference doesn't refer to a valid object
@@ -2150,14 +2173,10 @@ static int repack_ref_fn(struct ref_entry *entry, void
*cb_data)
* the output.
*/
error("%s does not point to a valid object!", entry->name);
+ string_list_append(refs_to_delete, entry->name);
return 0;
}
- peel_status = peel_entry(entry, 0);
- write_packed_entry(*fd, entry->name, entry->u.value.sha1,
- peel_status == PEEL_PEELED ?
- entry->u.value.peeled : NULL);
-
return 0;
}
@@ -2165,6 +2184,8 @@ static int repack_without_ref(const char *refname)
{
int fd;
struct ref_dir *packed;
+ struct string_list refs_to_delete = STRING_LIST_INIT_DUP;
+ struct string_list_item *ref_to_delete;
if (!get_packed_ref(refname))
return 0; /* refname does not exist in packed refs */
@@ -2185,8 +2206,13 @@ static int repack_without_ref(const char *refname)
rollback_lock_file(&packlock);
return 0;
}
+ do_for_each_entry_in_dir(packed, 0, curate_packed_ref_fn,
&refs_to_delete);
+ for_each_string_list_item(ref_to_delete, &refs_to_delete) {
+ if (remove_entry(packed, ref_to_delete->string) == -1)
+ die("internal error");
+ }
write_or_die(fd, PACKED_REFS_HEADER, strlen(PACKED_REFS_HEADER));
- do_for_each_entry_in_dir(packed, 0, repack_ref_fn, &fd);
+ do_for_each_entry_in_dir(packed, 0, write_packed_
[PATCH 05/12] refs: manage lifetime of packed refs cache via reference counting
In struct packed_ref_cache, keep a count of the number of users of the
data structure. Only free the packed ref cache when the reference
count goes to zero rather than when the packed ref cache is cleared.
This mechanism will be used to prevent the cache data structure from
being freed while it is being iterated over.
So far, only the reference in struct ref_cache::packed is counted;
other users will be adjusted in separate commits.
Signed-off-by: Michael Haggerty
---
refs.c | 39 ---
1 file changed, 36 insertions(+), 3 deletions(-)
diff --git a/refs.c b/refs.c
index 114bd5b..b0d77a7 100644
--- a/refs.c
+++ b/refs.c
@@ -806,6 +806,14 @@ struct packed_ref_cache {
struct ref_entry *root;
/*
+* Count of references to the data structure in this instance,
+* including the pointer from ref_cache::packed if any. The
+* data will not be freed as long as the reference count is
+* nonzero.
+*/
+ unsigned int referrers;
+
+ /*
* Iff the packed-refs file associated with this instance is
* currently locked for writing, this points at the associated
* lock (which is owned by somebody else).
@@ -832,14 +840,38 @@ static struct ref_cache {
char name[1];
} ref_cache, *submodule_ref_caches;
+/*
+ * Increment the reference count of *packed_refs.
+ */
+static void acquire_packed_ref_cache(struct packed_ref_cache *packed_refs)
+{
+ packed_refs->referrers++;
+}
+
+/*
+ * Decrease the reference count of *packed_refs. If it goes to zero,
+ * free *packed_refs and return true; otherwise return false.
+ */
+static int release_packed_ref_cache(struct packed_ref_cache *packed_refs)
+{
+ if (!--packed_refs->referrers) {
+ free_ref_entry(packed_refs->root);
+ free(packed_refs);
+ return 1;
+ } else {
+ return 0;
+ }
+}
+
static void clear_packed_ref_cache(struct ref_cache *refs)
{
if (refs->packed) {
- if (refs->packed->lock)
+ struct packed_ref_cache *packed_refs = refs->packed;
+
+ if (packed_refs->lock)
die("internal error: packed-ref cache cleared while
locked");
- free_ref_entry(refs->packed->root);
- free(refs->packed);
refs->packed = NULL;
+ release_packed_ref_cache(packed_refs);
}
}
@@ -1020,6 +1052,7 @@ static struct packed_ref_cache
*get_packed_ref_cache(struct ref_cache *refs)
FILE *f;
refs->packed = xcalloc(1, sizeof(*refs->packed));
+ acquire_packed_ref_cache(refs->packed);
refs->packed->root = create_dir_entry(refs, "", 0, 0);
refs->packed->fd = -1;
if (*refs->name)
--
1.8.3
--
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 03/12] refs: wrap the packed refs cache in a level of indirection
As we know, we can solve any problem in this manner. In this case,
the problem is to avoid freeing a packed refs cache while somebody is
using it. So add a level of indirection as a prelude to
reference-counting the packed refs cache.
Signed-off-by: Michael Haggerty
---
refs.c | 32 ++--
1 file changed, 26 insertions(+), 6 deletions(-)
diff --git a/refs.c b/refs.c
index bb3640f..4cee36b 100644
--- a/refs.c
+++ b/refs.c
@@ -802,6 +802,10 @@ static int is_refname_available(const char *refname, const
char *oldrefname,
return 1;
}
+struct packed_ref_cache {
+ struct ref_entry *root;
+};
+
/*
* Future: need to be in "struct repository"
* when doing a full libification.
@@ -809,7 +813,7 @@ static int is_refname_available(const char *refname, const
char *oldrefname,
static struct ref_cache {
struct ref_cache *next;
struct ref_entry *loose;
- struct ref_entry *packed;
+ struct packed_ref_cache *packed;
/*
* The submodule name, or "" for the main repo. We allocate
* length 1 rather than FLEX_ARRAY so that the main ref_cache
@@ -821,7 +825,8 @@ static struct ref_cache {
static void clear_packed_ref_cache(struct ref_cache *refs)
{
if (refs->packed) {
- free_ref_entry(refs->packed);
+ free_ref_entry(refs->packed->root);
+ free(refs->packed);
refs->packed = NULL;
}
}
@@ -992,24 +997,39 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)
}
}
-static struct ref_dir *get_packed_refs(struct ref_cache *refs)
+/*
+ * Get the packed_ref_cache for the specified ref_cache, creating it
+ * if necessary.
+ */
+static struct packed_ref_cache *get_packed_ref_cache(struct ref_cache *refs)
{
if (!refs->packed) {
const char *packed_refs_file;
FILE *f;
- refs->packed = create_dir_entry(refs, "", 0, 0);
+ refs->packed = xcalloc(1, sizeof(*refs->packed));
+ refs->packed->root = create_dir_entry(refs, "", 0, 0);
if (*refs->name)
packed_refs_file = git_path_submodule(refs->name,
"packed-refs");
else
packed_refs_file = git_path("packed-refs");
f = fopen(packed_refs_file, "r");
if (f) {
- read_packed_refs(f, get_ref_dir(refs->packed));
+ read_packed_refs(f, get_ref_dir(refs->packed->root));
fclose(f);
}
}
- return get_ref_dir(refs->packed);
+ return refs->packed;
+}
+
+static struct ref_dir *get_packed_ref_dir(struct packed_ref_cache
*packed_ref_cache)
+{
+ return get_ref_dir(packed_ref_cache->root);
+}
+
+static struct ref_dir *get_packed_refs(struct ref_cache *refs)
+{
+ return get_packed_ref_dir(get_packed_ref_cache(refs));
}
void add_packed_ref(const char *refname, const unsigned char *sha1)
--
1.8.3
--
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 v4 1/2] status: introduce status.short to enable --short by default
jorge-juan.garcia-gar...@ensimag.imag.fr writes: > diff --git a/t/t7508-status.sh b/t/t7508-status.sh > index e2ffdac..3c0818b 100755 > --- a/t/t7508-status.sh > +++ b/t/t7508-status.sh > @@ -1335,4 +1335,39 @@ test_expect_failure '.git/config ignore=all suppresses > submodule summary' ' > git config -f .gitmodules --remove-section submodule.subname > ' > > +test_expect_success 'Setup of test environment' ' > + git config status.showUntrackedFiles no > +' > + > +test_expect_success '"status.short=true" same as "-s"' ' > + git -c status.short=true status >actual && > + git status -s >expected_short && > + test_cmp expected_short actual > +' > + > +test_expect_success '"status.short=true" different from "--no-short"' ' > + git status --no-short >expected_noshort && > + test_must_fail test_cmp expected_noshort actual > +' I am not sure if "must be different, and I do not care what kind of difference we get" is a good test. > +test_expect_success '"status.short=true" weaker than "--no-short"' ' > + git -c status.short=true status --no-short >actual && > + test_cmp expected_noshort actual > +' > + > +test_expect_success '"status.short=false" same as "--no-short"' ' > + git -c status.short=false status >actual && > + git status -s >expected_short && > + test_cmp expected_noshort actual > +' I think the second line in this test is unwanted. > + > +test_expect_success '"status.short=false" weaker than "-s"' ' > + git -c status.short=false status -s >actual && > + test_cmp expected_short actual > +' > + > +test_expect_success 'Restore default test environment' ' > + git config --unset status.showUntrackedFiles > +' > + > test_done I'll queue this patch after tweaking the test part like this. Thanks. diff --git a/t/t7508-status.sh b/t/t7508-status.sh index e2ffdac..33cadd0 100755 --- a/t/t7508-status.sh +++ b/t/t7508-status.sh @@ -1335,4 +1335,34 @@ test_expect_failure '.git/config ignore=all suppresses submodule summary' ' git config -f .gitmodules --remove-section submodule.subname ' +test_expect_success 'setup of test environment' ' + git config status.showUntrackedFiles no && + git status -s >expected_short && + git status --no-short >expected_noshort +' + +test_expect_success '"status.short=true" same as "-s"' ' + git -c status.short=true status >actual && + test_cmp expected_short actual +' + +test_expect_success '"status.short=true" weaker than "--no-short"' ' + git -c status.short=true status --no-short >actual && + test_cmp expected_noshort actual +' + +test_expect_success '"status.short=false" same as "--no-short"' ' + git -c status.short=false status >actual && + test_cmp expected_noshort actual +' + +test_expect_success '"status.short=false" weaker than "-s"' ' + git -c status.short=false status -s >actual && + test_cmp expected_short actual +' + +test_expect_success 'Restore default test environment' ' + git config --unset status.showUntrackedFiles +' + test_done -- 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/RFC] git-remote-mediawiki: new tool to preview local changes without pushing
On Tue, Jun 11, 2013 at 11:31:31PM +0200, Benoît Person wrote: > I've implemented this one for now but after a real-life meeting with > Matthieu Moy we discussed the possibility to build a GitMediawiki.pm > module. It seems more "clean" than the concatenation of perl scripts. > Plus, it would force people to limit side effects inside the functions > used in this package/utils file (I have in mind the mw_connect_maybe > function here and a couple of others which directly *hope* for global > vars to be set to a nice value before being called). > > What I find bad in the concatenating-thingy is the mandatory rename of > git-mw.perl into something like git-mw.unmerged.perl and > git-remote-mediawiki.perl into git-remote-mediawiki.unmerged.perl. > Otherwise, like you said, it would be hard to chain to git's Makefile > after the merge. Yeah, I agree it's very hacky. > For now, I have really no idea which one is the best. If I may ask, > what did you have in mind while saying: > > > You could make a Git::MediaWiki.pm module, but installing that would > > significantly complicate the build procedure, and potentially be > > annoying for users. > > Since my previous commit (ea07ec1 in next - use Git.pm functions for > credentials), git-remote-mediawiki.perl already depends on the proper > installation of the Git.pm package. In what ways the need for the > installation of yet another package (GitMediawiki.pm) would annoy a > user ? I was thinking that you would be self-contained inside the contrib/mw-to-git directory, and therefore you would have to teach your code how to install the Git module, and you could not longer just "cp git-remote-mediawiki" into the right place to install it. But I think we have already crossed that bridge somewhat with Git.pm. And if you add your module as perl/Git/MediaWiki.pm and use the existing perl build system, then it is not any extra effort from the build system. That is probably the most sane way to go. -Peff -- 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/RFC] git-remote-mediawiki: new tool to preview local changes without pushing
The V2 is on the launchpad but I am still struggling with the code
factoring between git-mw.perl and git-remote-mediawiki.perl :/ .
On 9 June 2013 08:08, Jeff King wrote:
>
> You could make a Git::MediaWiki.pm module, but installing that would
> significantly complicate the build procedure, and potentially be
> annoying for users. One trick I have done in the past is to concatenate
> bits of perl script together in the Makefile, like this:
>
> foo: common.pl foo.pl
> { \
> echo '$(PERL_PATH_SQ)' && \
> for i in $^; do \
> echo "#line 1 $src" && \
> cat $src \
> done \
> } >$@+
> mv $@+ $@
>
> That would conflict a bit with the way we chain to git's Makefile,
> though. I suspect you could do something complicated like build "foo.pl"
> from "common.pl" and "foo-main.pl", then chain to git's Makefile to
> build "foo" from "foo.pl".
I've implemented this one for now but after a real-life meeting with
Matthieu Moy we discussed the possibility to build a GitMediawiki.pm
module. It seems more "clean" than the concatenation of perl scripts.
Plus, it would force people to limit side effects inside the functions
used in this package/utils file (I have in mind the mw_connect_maybe
function here and a couple of others which directly *hope* for global
vars to be set to a nice value before being called).
What I find bad in the concatenating-thingy is the mandatory rename of
git-mw.perl into something like git-mw.unmerged.perl and
git-remote-mediawiki.perl into git-remote-mediawiki.unmerged.perl.
Otherwise, like you said, it would be hard to chain to git's Makefile
after the merge.
For now, I have really no idea which one is the best. If I may ask,
what did you have in mind while saying:
> You could make a Git::MediaWiki.pm module, but installing that would
> significantly complicate the build procedure, and potentially be
> annoying for users.
?
Since my previous commit (ea07ec1 in next - use Git.pm functions for
credentials), git-remote-mediawiki.perl already depends on the proper
installation of the Git.pm package. In what ways the need for the
installation of yet another package (GitMediawiki.pm) would annoy a
user ?
Thank you for your time,
Benoit
--
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 0/4] Fix a race condition when reading loose refs
I've read the series while on a bus and found all of them sensible. I do share the worry of retry storm you expressed in the last one, and I agree that giving up after N times is a reasonable way out, when it becomes necessary. 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] Documentation/CommunityGuidelines
Jeff King writes: > So there are no hard rules, and this is not a democracy[1]. For the most > part the community runs itself in an open and collective fashion, and > the dictator's job is easy; but ultimately, he or she is in charge of > what gets applied and what doesn't. Rules like "break ties in favor of > reviewers" are just a guideline for the dictator to use in making > decisions. > > I do not think any of that is news to you, but I think the point needs > to be made, as it applies to any concrete rules. My original draft had "I am hoping we do not have to come to that" after "(I heard some communities break ties this way)", but I removed it by mistake. And I think you are right. I also am hoping that I am being fair to dictate ;-) > -Peff > > [1] Note that I think a benevolent dictator is a _terrible_ way to run a > real government, but it works in an open source project. I think the > difference is that dictatorship is open to abuse of power. In the > real world, there is a lot of power to abuse, and it is hard for > people to opt out of it. In the open source world, there is not that > much power, and if there is a bad dictator everyone can go somewhere > else (another project, or even a fork). So while a dictator _can_ > play favorites, or start deciding which patches to take based on > what they had for breakfast, there is a real incentive to remain > fair and reasonable. -- 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] Documentation/CommunityGuidelines
On Tue, Jun 11, 2013 at 10:00:56AM -0700, Junio C Hamano wrote: > > * Accept reviewers' comments gratefully and take them very seriously. > > Show that you appreciate the help by giving the reviewer the benefit of > > the doubt. If, after careful consideration, you find that you cannot > > agree with a reviewer's suggestion, explain your reasoning carefully > > without taking or giving offense, and seek compromise. > > In short, the only acceptable response to review comments are "You > are right. Here is a reroll", or "I think your suggestion will miss > these cases which I wanted to cover and that is why I did it this > way". There may be other small variants of the above two, but I > think I can agree with the general principle. > > In cases, there are two or more equally valid approaches to solving > a problem. I do not think I had to accept (or reject) many "it can > be done better in different ways and this perhaps is not the best > one" (or "it could be argued that it is correct") borderline topics > in the recent past, but I suspect that "a disagreement is healthy" > has to be accompanied by how disagreements that do not resolve > themselves are resolved (I think I've heard somewhere that some > communities break ties in favor of reviewers, for example). I more or less agree with what both of you have said above. The "ties goes to reviewers" thing I would be very wary of, at least as a hard rule. We do not (and do not want to) put any restrictions on who is allowed to do review. That sometimes results in unhelpful or even wrong reviews by new people, but those reviews are a stepping stone to being a more experienced and capable reviewer. Most of the time such reviews are resolved by other community members joining the discussion and coming to some agreement, but not always. And that is not even getting into the cases where long-time experienced reviewers are simply wrong or misguided, or the issue is legitimately a difficult tradeoff to consider, and the discussion ends in a stalemate. And I think that is where the benevolent dictator role comes in. They weigh not just the points made in the discussion (or a summary of it), but also use their judgement on who is making comments (how many people, the utility of their past comments) and other factors (other things happening in the project, being conservative because of recent mistakes made, etc). They may break such a tie by applying or rejecting, even by putting off a decision to revisit later (which is a de facto reject, of course). So there are no hard rules, and this is not a democracy[1]. For the most part the community runs itself in an open and collective fashion, and the dictator's job is easy; but ultimately, he or she is in charge of what gets applied and what doesn't. Rules like "break ties in favor of reviewers" are just a guideline for the dictator to use in making decisions. I do not think any of that is news to you, but I think the point needs to be made, as it applies to any concrete rules. -Peff [1] Note that I think a benevolent dictator is a _terrible_ way to run a real government, but it works in an open source project. I think the difference is that dictatorship is open to abuse of power. In the real world, there is a lot of power to abuse, and it is hard for people to opt out of it. In the open source world, there is not that much power, and if there is a bad dictator everyone can go somewhere else (another project, or even a fork). So while a dictator _can_ play favorites, or start deciding which patches to take based on what they had for breakfast, there is a real incentive to remain fair and reasonable. -- 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] instaweb: make the perl path configurable
It is convenient for the user to be able to customize the path to perl if they
do not want to use the system perl. This may be the case, for example, if the
user wants to use the plackup httpd but its extra dependencies are not
installed in the system perl; they can set the perl path to a perl that they
install and have control over in their own home directory.
Signed-off-by: Charles McGarvey
---
Documentation/config.txt | 4
git-instaweb.sh | 4 +++-
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 6e53fc5..e103594 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1549,6 +1549,10 @@ instaweb.modulepath::
instead of /usr/lib/apache2/modules. Only used if httpd
is Apache.
+instaweb.perlpath::
+ The path to the perl executable used by linkgit:git-instaweb[1] to
+ run gitweb and/or verify that the HTTP daemon is running.
+
instaweb.port::
The port number to bind the gitweb httpd to. See
linkgit:git-instaweb[1].
diff --git a/git-instaweb.sh b/git-instaweb.sh
index 01a1b05..8cfbdf2 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -3,7 +3,6 @@
# Copyright (c) 2006 Eric Wong
#
-PERL='@@PERL@@'
OPTIONS_KEEPDASHDASH=
OPTIONS_SPEC="\
git instaweb [options] (--start | --stop | --restart)
@@ -26,9 +25,12 @@ local="$(git config --bool --get instaweb.local)"
httpd="$(git config --get instaweb.httpd)"
root="$(git config --get instaweb.gitwebdir)"
port=$(git config --get instaweb.port)
+perl_path="$(git config --get instaweb.perlpath)"
module_path="$(git config --get instaweb.modulepath)"
action="browse"
+PERL=${perl_path:-@@PERL@@}
+
conf="$GIT_DIR/gitweb/httpd.conf"
# Defaults:
--
1.8.1.5
--
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 v3 07/28] git-remote-mediawiki: Rewrite unclear line of instructions
Le 11/06/2013 20:09, Junio C Hamano a écrit : > Matthieu Moy writes: > >>> my ($namespace) = @_; >>> my $namespace = shift; >>> >>> My impression has been that both are equally common, >> >> The second is the most common in git-remote-mediawiki (but I don't have >> any preference nor know what is recommended elsewhere). > > I wasn't implying I prefer the former. I was just being curious, > and if the latter is more prevalent in the code _and_ Perlcritique > does not have any issue with it, it is perfectly fine. Perlcritic doesn't have an issue with any of both cases. I think the second method is clearer when there is only one argument, because it makes it clear that there is only one. -- Célestin Matte -- 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] build: get rid of the notion of a git library
On Tue, Jun 11, 2013 at 2:59 PM, Junio C Hamano wrote: > Linus Torvalds writes: > >> This whole thread has been one long argument about totally pointless >> things that wouldn't improve anything one way or the other. It's >> bikeshedding of the worst kind. Just let it go. > > The proposal to move sequencer.c to builtins/sequencer.c and then > adding a filter in Makefile to exclude so that "git-sequencer" is > not built is "it wouldn't improve anything one way or the other". > It is to throw in something into a set to which it does not belong, In your opinion. > and then working around that mistake with another kludge. In your opinion. You continually use absolutist rhetoric to try to convince yourself and others that what you say is absolute 100% fact. But it's not, it's your opinion. > So I do not think this is not even a bikeshedding. Just one side > being right, and the other side continuing to repeat nonsense > without listening. George W. Bush said history would prove him right, but saying so doesn't make it so. At least he had the decency to acknowledge that other people had different valid opinions. builtin/lib.a makes perfect sense, and it's the first logical step in moving libgit.a towards libgit2. -- 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] build: get rid of the notion of a git library
Linus Torvalds writes: > This whole thread has been one long argument about totally pointless > things that wouldn't improve anything one way or the other. It's > bikeshedding of the worst kind. Just let it go. The proposal to move sequencer.c to builtins/sequencer.c and then adding a filter in Makefile to exclude so that "git-sequencer" is not built is "it wouldn't improve anything one way or the other". It is to throw in something into a set to which it does not belong, and then working around that mistake with another kludge. The problem that triggered the wrong solution actually is real, however. A function that sequencer.c (in libgit.a so that it could be used by standalone) may want to use in the future currently lives in builtin/notes.c. If you add a call to that function to sequencer.c without doing anything else, standalones like git-upload-pack will stop linking correctly. The git-upload-pack wants the revision traversal machinery in revision.o, which in turn wants to be able to see log-tree.o, which in turn wants to link with sequencer.o to see one global variable (there may be other dependencies). All of these objects are currently in libgit.a so that both builtins and standalones can use them. 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(). 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. So I do not think this is not even a bikeshedding. Just one side being right, and the other side continuing to repeat nonsense without listening. -- 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] Documentation/CommunityGuidelines
On Tue, Jun 11, 2013 at 7:40 AM, Michael Haggerty wrote: > At the risk of being > presumptuous myself, I suggest that you show a copy of your email to > somebody whom you know and respect in the real world, somebody who is > not immersed in the Git community meltdown. For example, somebody like > your mother or father, or a teacher whom you respect, or a member of > clergy if you are so inclined. Ask that person's opinion about your email. > > It is so easy to lose perspective in the Internet. Such excellent advice. Even if the advice is not taken literally, it is probably enough to just imagine how that person whom you respect would respond to the words in your emails. I am sure I do not do this enough in my own communications. I just wanted to draw attention to this wonderful suggestion again. Sometimes it is necessary to take a step back when discussions get heated, to regain perspective. -Brandon -- 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] Documentation/CommunityGuidelines
On Wed, Jun 12, 2013 at 12:16:28AM +0530, Ramkumar Ramachandra wrote: > John Keeping wrote: > > Ugh, why this roundabout-passive-past tone? Use imperative tone > > like this: > > > > ... > > > > vs. > > > > We normally use the imperative in commit messages, perhaps like > > this? > > > > ... > > > > As my mother would say, "politeness costs nothing" ;-) > > The review is being honest about her feelings in the first one, and > being artificially diplomatic in the second one. I don't think it is artificially diplomatic, it's an attempt to convey a helpful tone in an email. As has been said elsewhere, it is easy to read an email in the wrong tone (there is an oft-cited statistic about the percentage of communication that is non-verbal, and which cannot be inferred from written text). For this reason I think it is important for reviewers to make an effort to minimise the risk that what they write can be interpreted as being aggressive. > Both of them are > constructive and friendly, in that they provide an example for the > submitter to follow. Both provide the same advice, yes. But I disagree that they are both friendly. The top example reads (to me at least) as an attack on the submitter for not knowing better. It may sometimes be necessary to resort to strong wording if someone appears to be wilfully ignoring sensible advice but we should not expect every submitter to know the expectations of the project; the first message to someone should gently guide them in the right direction. > Either way, I'm not interested in problems that have no solutions. > The only "solution" I see here is to suffocate every contributor until > they are "tactful enough" for the majority's liking, and "remove" the > ones that don't conform. If you do have an alternate solution, please > share it with us. I don't have a solution, only a hope that regular contributors will learn from others how they can phrase review comments less aggressively. I expect different people will read the same statement differently; people are from different cultures and what is considered acceptable in one culture can be considered rude in another. We should aim to cultivate our own culture where we try to minimise the risk that what we write will be misinterpreted by someone with a different cultural background. -- 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] build: get rid of the notion of a git library
On Tue, Jun 11, 2013 at 2:24 PM, Junio C Hamano wrote: > Felipe Contreras writes: > >> On Tue, Jun 11, 2013 at 1:17 PM, Junio C Hamano wrote: >>> Felipe Contreras writes: >>> Moreover, if you are going to argue that we shouldn't be closing the door, then why not link ./builtin/*.o to libgit.a? >>> >>> Huh? It does not make any sense. builtin/*.o files have cmd_foo() >>> that are expected to be called from git.c::main(), but libgit.a >>> files are linked with no constraints whose main() they are linking >>> with. >> ... >>> That is exactly why I said that builtin/*.o should be refactored to >>> pick "does not have to be in builtin" bits, which will result in a >>> better division of labor. Reusable bits should live in the library, >>> while a particular implementation of command remain in builtin/* >>> that utilize the reusable bits. >>> >>> You still haven't justified why we have to _forbid_ any outside >>> callers from calling copy_notes_for_rewrite(). >> >> Because only builtins _should_ use it. > > And there is no justification behind that "_should_" claim; you are > not making any technical argument to explain it. The first argument of init_copy_notes_for_rewrite() is the name of the builtin command. There hardly could be any more justification. >> I asked you for an example, and >> you said a hypothetical standalone 'git-filter-branch' might use it, > > Of course it has to be hypothetical; I already said with the current > code no standalone does use it---it is not arranged to be doable so > there is no user. If you want to have examples of future possible > callers, they have to be hypothetical---the future by definition > hasn't happened. But that does not mean hypothetical is impractical > nor useless. So? It's still hypothetical, which is what I said. What are you complaining about? About the fact that I made a correct assessment? > There are out-of-tree programs like cgit that will not be built-in > but already link with libgit.a. Moving things that can be used by > outside people out of builtin/*.o to libgit.a would allow uses that > you and I cannot imagine offhand. I do not see a reason for us to > forbid a filter-branch replacement out of tree as a standalone. Yeah, I already ran that argument, and you conveniently chose to escape the next logical conclusion that I already put forward: --- a/Makefile +++ b/Makefile @@ -990,6 +990,8 @@ BUILTIN_OBJS += builtin/verify-pack.o BUILTIN_OBJS += builtin/verify-tag.o BUILTIN_OBJS += builtin/write-tree.o +LIB_OBJS += $(BUILTIN_OBJS) + GITLIBS = $(LIB_FILE) $(XDIFF_LIB) EXTLIBS = I don't think that's the right direction. > I do not see a point in continuing to discuss this (or any design > level issues) with you. You seem to go into a wrong direction to > break the design of the overall system, not in a direction to > improve anything. I do not know, and at this point I do not care, > if you are doing so deliberately to sabotage Git. Just stop. That's your opinion, and it's not shared by others (outside of the Git project). If you were right and Git was moving in the right direction, there would be no need for libgit2. -- 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] Documentation/CommunityGuidelines
From: "Michael Haggerty" Sent: Tuesday, June 11, 2013 7:52 PM [...] That's a very good point (and a good illustration, too). How do you like the new second and third sentences below? * When reviewing other peoples' code, be tactful and constructive. Remember that submitting patches for public critique can be very intimidating I found this to be true. The tone on the list could at times feel un-helpful (to the new person). It is almost as if it is an initiation - those on the list know the protocols, and new folk either arrive like a bull in a china shop, or more likely, timidly push the patch under the door and run away (and variations in between) - some never push out their (drafted) patch. and when mistakes are found it can be embarrassing. Sometimes it isn't 'mistakes', rather it is simply a lack of sufficient explanation to communicate intent, which may not have been understood by the reviewer/responder. In such cases it can be a frustration to know what was meant in the response, especially if the response is terse. [i.e. I think it would be reasonable to squeeze part of this in here somewhere to guide new contributors about this step] There is separately a need to note the role of the maintainer, who has a more difficult role as gatekeeper who's higher standards in applying the precautionary principle http://en.wikipedia.org/wiki/Precautionary_principle can feel like unhelpfulness, or worse if misunderstood. Do what you can to make it a positive and pleasant experience for the submitter. Set high expectations, but do what you can to help the submitter achieve them. Don't demand changes based only on your personal preferences. Don't let the perfect be the enemy of the good. (As Junio pointed out, the last sentence is not so great and a better replacement would be welcome.) As my mother would say, "politeness costs nothing" ;-) Does your mother program C? We could use her around here :-) I think she programmed in Smalltalk and CleanYourRoom. (sorry not my question ;-) Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ regards Philip -- 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] Documentation/CommunityGuidelines
On Tue, Jun 11, 2013 at 2:06 PM, Junio C Hamano wrote: > Ramkumar Ramachandra writes: >> I'm sorry, but the problem has no solution then. >> >> The "problem" we are dealing with is irrational and/or out-of-tone >> emails. Unless you possess some mind-control mechanism that will get >> all contributors to write emails that conform to your standards, there >> is no solution. > > Actually there is. Just ignore the troll. Congratulations Junio. You have followed our drafted guidelines by choosing to lead by example how not to not propagate the violence, turn around, and take the high road. After kicking your opponent in the groin. -- 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] Documentation/CommunityGuidelines
On Tue, Jun 11, 2013 at 1:29 PM, John Keeping wrote: > I realise that we shouldn't take offence to review comments, but we are > all human and it is sometimes hard not to take things personally. > > In the examples above, the first makes it feel like the submitter is > fighting to get a patch included, but the second feels like we're > collaborating to get to the best result for the project. > > As my mother would say, "politeness costs nothing" ;-) That's right, I agree with everything you said, but what would your mother say about the people are not polite towards you? I doubt it would be "fuck them then". You should be polite, you should not demand politeness. Being polite towards the people that are polite to you barely has any merit, swallowing your pride, taking a deep breath, trying to understand that the other side might be just having a bad day, or any number of reasons for the impoliteness... that's what takes effort. Escalating violence is easy, blaming the other side for starting is also easy (as any toddler would tell you), being the side that puts the other cheek is what's hard. -- 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] build: get rid of the notion of a git library
Felipe Contreras writes: > On Tue, Jun 11, 2013 at 1:17 PM, Junio C Hamano wrote: >> Felipe Contreras writes: >> >>> Moreover, if you are going to argue that we shouldn't be closing the >>> door, then why not link ./builtin/*.o to libgit.a? >> >> Huh? It does not make any sense. builtin/*.o files have cmd_foo() >> that are expected to be called from git.c::main(), but libgit.a >> files are linked with no constraints whose main() they are linking >> with. > ... >> That is exactly why I said that builtin/*.o should be refactored to >> pick "does not have to be in builtin" bits, which will result in a >> better division of labor. Reusable bits should live in the library, >> while a particular implementation of command remain in builtin/* >> that utilize the reusable bits. >> >> You still haven't justified why we have to _forbid_ any outside >> callers from calling copy_notes_for_rewrite(). > > Because only builtins _should_ use it. And there is no justification behind that "_should_" claim; you are not making any technical argument to explain it. > I asked you for an example, and > you said a hypothetical standalone 'git-filter-branch' might use it, Of course it has to be hypothetical; I already said with the current code no standalone does use it---it is not arranged to be doable so there is no user. If you want to have examples of future possible callers, they have to be hypothetical---the future by definition hasn't happened. But that does not mean hypothetical is impractical nor useless. There are out-of-tree programs like cgit that will not be built-in but already link with libgit.a. Moving things that can be used by outside people out of builtin/*.o to libgit.a would allow uses that you and I cannot imagine offhand. I do not see a reason for us to forbid a filter-branch replacement out of tree as a standalone. I do not see a point in continuing to discuss this (or any design level issues) with you. You seem to go into a wrong direction to break the design of the overall system, not in a direction to improve anything. I do not know, and at this point I do not care, if you are doing so deliberately to sabotage Git. Just stop. -- 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] Documentation/CommunityGuidelines
On Tue, Jun 11, 2013 at 08:52:05PM +0200, Michael Haggerty wrote: > That's a very good point (and a good illustration, too). How do you > like the new second and third sentences below? > > * When reviewing other peoples' code, be tactful and constructive. > Remember that submitting patches for public critique can be very > intimidating and when mistakes are found it can be embarrassing. Do > what you can to make it a positive and pleasant experience for the > submitter. Set high expectations, but do what you can to help the > submitter achieve them. Don't demand changes based only on your > personal preferences. Don't let the perfect be the enemy of the good. I'm not sure. I like the intent, but I'm not sure that it's clear enough that we're talking about the tone of comments rather than the type of feedback to provide. How about something like this? * Having your code reviewed should feel like a collaboration aiming for the best result for the project, not like a fight to get your patch accepted. Try to bear this in mind when reviewing other peoples' code and consider how you would feel reading the same comments if the review was the other way round. We are only human and the tone of a review can influence how the following discussion progresses. * If you do feel that a review is aggressive, don't reply immediately. Contributors are spread around the world in different timezones and it is often better to wait a few hours for others to comment before rushing to defend your patch. -- 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] build: get rid of the notion of a git library
On Tue, Jun 11, 2013 at 1:14 PM, Linus Torvalds wrote: > On Tue, Jun 11, 2013 at 11:06 AM, Felipe Contreras > wrote: >> >> Moreover, if you are going to argue that we shouldn't be closing the >> door [...] > > Felipe, you saying "if you are going to argue ..." to anybody else is > kind of ironic. Supposing the other side's argument is correct is a standard discussing technique. > Why is it every thread I see you in, you're being a dick and arguing > for some theoretical thing that nobody else cares about? I don't know. I've sent 800 patches in the last three months (patches, not email comments), and you pick this one to reply to. Maybe because you enjoy insulting people? > This whole thread has been one long argument about totally pointless > things that wouldn't improve anything one way or the other. It's > bikeshedding of the worst kind. Just let it go. Why don't you ask Junio to let it go? If it's irrelevant, than it doesn't matter if this patch is applied or not. You say it's bike-shedding, that implies that Junio likes red, and I like blue, and both are equally useless. So let's go for blue then. Presumably Junio doesn't agree with you, he does truly think it should be red, in fact, he doesn't think it's just a color, it's something important, and I agree, and apparently other people in the mailing list also agree, as most of them have voiced their opinion that red is the color. Now, do you have something of value to say which of the two options should be, or are you just going to engage in double standards and personal attacks? If you truly think this is bikeshedding, at least be fair and complain about that to the people that argue for red, not just the ones that argue for blue. -- 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] Documentation/CommunityGuidelines
Ramkumar Ramachandra writes: > Michael Haggerty wrote: >> I stopped reading your email here. I've read enough tactless emails >> over the last few days, but to be asked to read an email that was >> *intentionally* written tactlessly is too detrimental to my quality of life. > > I'm sorry, but the problem has no solution then. > > The "problem" we are dealing with is irrational and/or out-of-tone > emails. Unless you possess some mind-control mechanism that will get > all contributors to write emails that conform to your standards, there > is no solution. Actually there is. Just ignore the troll. In the past few days, I've learned to mostly skim mails from you and Felipe on this topic (and perhaps some other topics) just enough to see if there is anything worth reading and/or responding to in them, and have ignored most of them. That gave me some time back to do the real work. If you argue that we should not punish "people" but "bad behaviour", that is fine. The "From:" field, combined with the "Subject: " field, is often a pretty good indication to tell if a message is worth reading and/or responding to, so ignoring such messages and ignoring troll senders practically amount to the same thing. -- 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] build: get rid of the notion of a git library
On Tue, Jun 11, 2013 at 1:17 PM, Junio C Hamano wrote: > Felipe Contreras writes: > >> Moreover, if you are going to argue that we shouldn't be closing the >> door, then why not link ./builtin/*.o to libgit.a? > > Huh? It does not make any sense. builtin/*.o files have cmd_foo() > that are expected to be called from git.c::main(), but libgit.a > files are linked with no constraints whose main() they are linking > with. >> If you are >> seriously considering the highly unlikely hypothetical standalone >> git-filter-branch scenario, you should consider the even more likely >> scenario where somebody needs to access code from ./builtin/*.o; that >> scenario is not even hypothetical, we know it's happened multiple >> times, and we know it's going to happen again. > > That is exactly why I said that builtin/*.o should be refactored to > pick "does not have to be in builtin" bits, which will result in a > better division of labor. Reusable bits should live in the library, > while a particular implementation of command remain in builtin/* > that utilize the reusable bits. > > You still haven't justified why we have to _forbid_ any outside > callers from calling copy_notes_for_rewrite(). Because only builtins _should_ use it. I asked you for an example, and you said a hypothetical standalone 'git-filter-branch' might use it, but you have not explained why it should be standalone, when it's clear it should be a builtin. If we assume your argument is valid for the hypothetical 'git-filter-branch', if that's the case, then it would be even more reasonable to assume that there will be other standalone binaries that would want to use all sort of functions from ./builtin/*.o. Let's put an example: reset_index(). Some standalone program wants to use that function. What do you we do? The shortest route is to make it non-static, and add it to builtin.h. But that would not be enough, we need the infrastructure prepared for that; link libgit.a with ./builtin/*.o. I don't think that's the way to go, but your line of argumentation leads directly there; if we are worrying about anything that any potential standalone program could want, then we should worry about reset_index() not being easily accessible to them. IMO we should be clear and say no; standalone programs should not access copy_notes_for_rewrite(), that's for builtins. If we move all the code that potential standalone programs could want to libgit.a, it wouldn't be a library at all, and it would basically contain everything. -- 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: rebase --autosquash does not handle fixup! of fixup!
Andrew Pimlott writes:
> git rebase -i --autosquash does not handle a fixup! of a fixup!, such as
> the history:
>
> aaa fix nasty bug
> ...
> bbb fixup! fix nasty bug
> ...
> ccc fixup! fixup! fix nasty bug
>
> --autosquash produces:
>
> pick aaa fix nasty bug
> fixup bbb fixup! fix nasty bug
> ...
> pick ccc fixup! fixup! fix nasty bug
>
> This defeats the workflow I was hoping to use:
>
> git commit -m 'fix nasty bug'
> ...
> git commit --fixup :/nasty
> ...
> git commit --fixup :/nasty
>
> The second :/nasty resolves to the previous fixup, not the initial
> commit. I could have made the regular expression more precise, but this
> would be a hassle.
>
> Would a change to support fixup! fixup! be considered?
Sure, why not. You could start with something like the patch below
(untested). If that happens to work, just add a test and a good commit
message.
diff --git i/git-rebase--interactive.sh w/git-rebase--interactive.sh
index f953d8d..798ae81 100644
--- i/git-rebase--interactive.sh
+++ w/git-rebase--interactive.sh
@@ -689,7 +689,17 @@ rearrange_squash () {
case "$message" in
"squash! "*|"fixup! "*)
action="${message%%!*}"
- rest="${message#*! }"
+ rest=$message
+ while : ; do
+ case "$rest" in
+ "squash! "*|"fixup! "*)
+ rest="${rest#*! }"
+ ;;
+ *)
+ break
+ ;;
+ esac
+ done
echo "$sha1 $action $rest"
# if it's a single word, try to resolve to a full sha1
and
# emit a second copy. This allows us to match on both
message
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
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] Documentation/CommunityGuidelines
Michael Haggerty wrote: > I stopped reading your email here. I've read enough tactless emails > over the last few days, but to be asked to read an email that was > *intentionally* written tactlessly is too detrimental to my quality of life. I'm sorry, but the problem has no solution then. The "problem" we are dealing with is irrational and/or out-of-tone emails. Unless you possess some mind-control mechanism that will get all contributors to write emails that conform to your standards, there is no solution. If you feel strongly that everyone must conform to your standards, you must "remove" the members that do not conform to that standard. I have no desire to be suffocated to conform to your standard, so I'm ready to leave. -- 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] Documentation/CommunityGuidelines
On 06/11/2013 08:29 PM, John Keeping wrote: > On Tue, Jun 11, 2013 at 10:00:56AM -0700, Junio C Hamano wrote: >> Michael Haggerty writes: >>> * When reviewing other peoples' code, be tactful and constructive. Set >>> high expectations, but do what you can to help the submitter achieve >>> them. Don't demand changes based only on your personal preferences. >>> Don't let the perfect be the enemy of the good. >> >> I think this is 30% aimed at me (as I think I do about that much of >> the reviews around here). I fully agree with most of them, but the >> last sentence is a bit too fuzzy to be a practically useful >> guideline. Somebody's bare minimum is somebody else's perfection. >> An unqualified "perfect is the enemy of good" is often incorrectly >> used to justify "It works for me." and "There already are other >> codepaths that do it in the same wrong way.", both of which make >> things _worse_ for the long term project health. > > One thing that I think is missing from these proposals so far is some > clear indication that a review should not be confrontational. Consider > the following two review comments (taken from a recent example that > happened to stick in my mind, but I don't want to single out any one > individual here): > > Ugh, why this roundabout-passive-past tone? Use imperative tone > like this: > > ... > > vs. > > We normally use the imperative in commit messages, perhaps like > this? > > ... > > Both say the same thing but the first immediately puts the submitter on > the defensive. If I see something like that on one of my patches I have > to consciously resist the urge to reply immediately and instead review > what I'm about to send once I've calmed down. That's a very good point (and a good illustration, too). How do you like the new second and third sentences below? * When reviewing other peoples' code, be tactful and constructive. Remember that submitting patches for public critique can be very intimidating and when mistakes are found it can be embarrassing. Do what you can to make it a positive and pleasant experience for the submitter. Set high expectations, but do what you can to help the submitter achieve them. Don't demand changes based only on your personal preferences. Don't let the perfect be the enemy of the good. (As Junio pointed out, the last sentence is not so great and a better replacement would be welcome.) > As my mother would say, "politeness costs nothing" ;-) Does your mother program C? We could use her around here :-) Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- 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] Documentation/CommunityGuidelines
John Keeping wrote: > Ugh, why this roundabout-passive-past tone? Use imperative tone > like this: > > ... > > vs. > > We normally use the imperative in commit messages, perhaps like > this? > > ... > > As my mother would say, "politeness costs nothing" ;-) The review is being honest about her feelings in the first one, and being artificially diplomatic in the second one. Both of them are constructive and friendly, in that they provide an example for the submitter to follow. Either way, I'm not interested in problems that have no solutions. The only "solution" I see here is to suffocate every contributor until they are "tactful enough" for the majority's liking, and "remove" the ones that don't conform. If you do have an alternate solution, please share it with us. -- 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: Tracking vendor release with Git
From: "Yann Droneaud" Sent: Tuesday, June 11, 2013 6:06 PM Hi, I'm trying to setup a workflow to track vendor releases (upstream). Each new release are provided as an archive of source code, data, documentation, etc. For each vendor releases, fixes need to be applied before making them available to users (downstream). Seems to be a rather common use case, ... Have you looked at the msysgit process that has to cope with upstream git ;-) e.g. https://code.google.com/p/msysgit/source/browse/share/msysGit/merging-rebase.sh?name=python https://github.com/msysgit/msysgit/blob/master/share/msysGit/merging-rebase.sh and https://github.com/msysgit/msysgit/blob/master/share/msysGit/rebasing-merge.sh whereby the guys re-apply all the patches that haven't been accepted upstream, along with local fixups to get each new release working. Philip -- 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] Documentation/CommunityGuidelines
On 06/11/2013 08:16 PM, Ramkumar Ramachandra wrote: > This is an exercise. I can easily be more tactful (as evidenced by > other threads), but I'm choosing not to be. I want you to focus on > the argument, and not the tone. I stopped reading your email here. I've read enough tactless emails over the last few days, but to be asked to read an email that was *intentionally* written tactlessly is too detrimental to my quality of life. In German there is an expression "Der Ton macht die Musik": "the tone makes the music", by which they mean that the *way* something is said is at least as important as what is said. The tone *is* an integral part of the message and (1) the writer will be much more effective by making the tone agree with the literal words of the message and (2) for this particular reader, the effort of accommodating writers who are unwilling to do so has become too exhausting. I naively thought that I might be able to help calm the situation, but I have concluded that nothing I can say or do will have that effect. Therefore I bow out of this part of the conversation and hope either that it will fizzle out or that perhaps a deus ex machina will appear. Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- 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
rebase --autosquash does not handle fixup! of fixup!
git rebase -i --autosquash does not handle a fixup! of a fixup!, such as the history: aaa fix nasty bug ... bbb fixup! fix nasty bug ... ccc fixup! fixup! fix nasty bug --autosquash produces: pick aaa fix nasty bug fixup bbb fixup! fix nasty bug ... pick ccc fixup! fixup! fix nasty bug This defeats the workflow I was hoping to use: git commit -m 'fix nasty bug' ... git commit --fixup :/nasty ... git commit --fixup :/nasty The second :/nasty resolves to the previous fixup, not the initial commit. I could have made the regular expression more precise, but this would be a hassle. Would a change to support fixup! fixup! be considered? Andrew (Please Cc: me on replies.) -- 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: Exact format of tree objets
Chico Sokol writes: > Is there any official documentation of tree objets format? Are tree > objects encoded specially in some way? How can I parse the inflated > contents of a tree object? > > We're suspecting that there is some kind of special format or > encoding, because the command "git cat-file -p " show me ... > While "git cat-file tree " generate ... "cat-file -p" is meant to be human-readable form. The latter gives the exact byte contents read_sha1_file() sees, which is a binary format. Essentially, it is a sequence of: - mode of the entry encoded in octal, without any leading '0' pad; - pathname component of the entry, terminated with NUL; - 20-byte SHA-1 object name. sorted in a particular order. -- 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: Exact format of tree objets
On Tue, Jun 11, 2013 at 01:25:14PM -0300, Chico Sokol wrote: > Is there any official documentation of tree objets format? Are tree > objects encoded specially in some way? How can I parse the inflated > contents of a tree object? Tree object consists of entries, each concatenation of: - Octal mode (using ASCII digits 0-7). - Single SPACE (0x20) - Filename - Single NUL (0x00) - 20-byte binary SHA-1 of referenced object. At least following octal modes are known: 4: Directory (tree). 100644: Regular file (blob). 100755: Executable file (blob). 12: Symbolic link (blob). 16: Submodule (commit). The entries are always sorted in (bytewise) lexicographical order, except directories sort like there was impiled '/' at the end. So e.g.: ! < 0 < 9 < a < a- < a- (directory) < a (directory) < a0 < ab < b < z. The idea of sorting directories specially is that if one recurses upon hitting a directory and uses '/' as path separator, then the full filenames are in bytewise lexicographical order. -Ilari -- 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
