Re: fix french translation
Please do not forget to reply to all. On Mon, May 27, 2013 at 1:34 AM, 乙酸鋰 ch3co...@gmail.com wrote: git-gui/po/fr.po | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git b/git-gui/po/fr.po a/git-gui/po/fr.po index 0aff186..40441db 100644 --- b/git-gui/po/fr.po +++ a/git-gui/po/fr.po @@ -1139,7 +1139,7 @@ msgstr Standard (rapide, semi-redondant, liens durs) #: lib/choose_repository.tcl:514 msgid Full Copy (Slower, Redundant Backup) -msgstr Copie complète (plus lent, sauvegarde redondante) +msgstr Copy complète (plus lent, sauvegarde redondante) I still don't get why Copie is replaced by Copy ? -- 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] diff: add --ignore-blank-lines option
On Sun, May 26, 2013 at 10:35 PM, Johannes Sixt j...@kdbg.org wrote: Am 26.05.2013 19:58, schrieb Antoine Pelisse: The goal of the patch is to introduce the GNU diff -B/--ignore-blank-lines as closely as possible. The short option is not available because it's already used for break-rewrites. When this option is used, git-diff will not create hunks that simply adds or removes empty lines, but will still show empty lines addition/suppression if they are close enough to valuable changes. So when an addition or removal of a blank line appears in a hunk that also has non-blank-line changes, the addition or removal is not treated specially? Exactly. How is a blank line defined? What happens if a line that has only whitespace is added or removed? xdl_blankline() is the best description of what I considered a blank line. If no --ignore-space-* option is given, it's a line that starts and ends with '\n'. If any --ignore-space-* option is given, it's a line that has any number of isspace(3)-defined characters, followed by '\n'. I'm thinking of diffs of files with CRLF Good you did, because I didn't ;-) line breaks, where the CR would count as whitespace in the line, I think. With the current implementation, an empty line with CRLF will not show as a blank line if no space option is given. As CR is a space according to isspace(3), the line will be removed with any space option. +--ignore-blank-lines:: + Ignore changes whose lines are all blank. I think this is too terse and does not convey what the option really does. That's the description from GNU diff man page. But indeed it could be more precise. +test_expect_success 'ignore-blank-lines: only new lines' ' + seq 5 x Please use test_seq instead of seq in all new tests. Will fix. -- Hannes -- 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 v2] commit: don't use-editor when allow-empty-message
So now we have two fixes for the same issue, don't we ?
You probably missed $gmane/225534.
On Mon, May 27, 2013 at 4:20 PM, Ramkumar Ramachandra
artag...@gmail.com wrote:
Commit a24a41e (git-commit: only append a newline to -m mesg if
necessary, 2013-02-18) introduced a regression: when
--allow-empty-message is used and an empty message is explicitly
specified with -m , git commit still launches $EDITOR unnecessarily.
The commit (correctly) fixes opt_parse_m() to not fill in two newlines
into the message buffer unconditionally. The real problem is that
launching $EDITOR only depends on use_editor and whether message is
empty. Fix the problem by setting explicit_message in the codepath
where an explicit string is passed via -m, and then checking it before
launching $EDITOR.
Reported-by: Mislav Marohnić mislav.maroh...@gmail.com
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
Works?
builtin/commit.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index d2f30d9..7d72ba7 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -108,6 +108,7 @@ static const char *cleanup_arg;
static enum commit_whence whence;
static int use_editor = 1, include_status = 1;
static int show_ignored_in_status;
+static int explicit_message = 0;
static const char *only_include_assumed;
static struct strbuf message = STRBUF_INIT;
@@ -128,6 +129,7 @@ static int opt_parse_m(const struct option *opt, const
char *arg, int unset)
strbuf_addch(buf, '\n');
strbuf_addstr(buf, arg);
strbuf_complete_line(buf);
+ explicit_message = 1;
}
return 0;
}
@@ -824,7 +826,7 @@ static int prepare_to_commit(const char *index_file,
const char *prefix,
git_path(commit_editmsg), hook_arg1, hook_arg2, NULL))
return 0;
- if (use_editor) {
+ if (use_editor !explicit_message) {
char index[PATH_MAX];
const char *env[2] = { NULL };
env[0] = index;
--
1.8.3.1.g33669de.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
--
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 14/21] remote-hg: show more proper errors
Hey Felipe,
I know that has been integrated a while ago.
On Thu, Apr 11, 2013 at 2:23 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
When cloning or pushing fails, we don't want to show a stack-trace.
diff --git a/contrib/remote-helpers/git-remote-hg
b/contrib/remote-helpers/git-remote-hg
index ff89725..3ae3598 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -284,11 +284,17 @@ def get_repo(url, alias):
else:
local_path = os.path.join(dirname, 'clone')
if not os.path.exists(local_path):
-peer, dstpeer = hg.clone(myui, {}, url, local_path,
update=False, pull=True)
+try:
+peer, dstpeer = hg.clone(myui, {}, url, local_path,
update=True, pull=True)
+except:
+die('Repository error')
repo = dstpeer.local()
Can you explain why update went from False to True ? That can be a
problem if the repository is BIG (two working directories instead of
one can raise space issues).
The commit message is not so helpful here ;)
--
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: fix french translation
Hi, Patch should be inlined, please have a look at `Documentation/SubmittingPatches`. Also, how is copy (an english word) better than copie (the literal french translation) ? On Sun, May 26, 2013 at 5:27 PM, 乙酸鋰 ch3co...@gmail.com wrote: see 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 1/3] for-each-ref: introduce %C(...) for color
On Sat, May 25, 2013 at 1:50 PM, Ramkumar Ramachandra artag...@gmail.com wrote: Antoine Pelisse wrote: Is it not possible for color to be used uninitialized here ? My compiler didn't complain; what am I missing? Doesn't the declaration char color[COLOR_MAXLEN]; initialize an empty string? As John said, this is allocated on the stack. What do you want it to be initialized to ? Filled with zeros ? That's overkill because only the first char needs to be zeroed. The compiler can't know what to do and it lets you to take the best action. More importantly, aren't there numerous instances of this in the codebase? I think Valgrind would be mad at us if there was many instances of this. By the way Ramkumar, could you check if Valgrind complains with 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: [QUERY] How do you sort completions?
On Fri, May 24, 2013 at 6:18 PM, Ramkumar Ramachandra artag...@gmail.com wrote: Damn; so it's impossible to have a custom-sorted completion list in bash. Any idea about zsh? I know that there are completion groups, but I'd really like custom sorting. I think sorting is required for faster look-up, most likely with dichotomic search. Otherwise it would have to search the whole list each time. -- 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 1/3] for-each-ref: introduce %C(...) for color
On Fri, May 24, 2013 at 4:19 PM, Ramkumar Ramachandra
artag...@gmail.com wrote:
@@ -928,12 +936,22 @@ static void emit(const char *cp, const char *ep)
static void show_ref(struct refinfo *info, const char *format, int
quote_style)
{
const char *cp, *sp, *ep;
+ char color[COLOR_MAXLEN];
for (cp = format; *cp (sp = find_next(cp)); cp = ep + 1) {
ep = strchr(sp, ')');
if (cp sp)
emit(cp, sp);
- print_value(info, parse_atom(sp + 2, ep), quote_style);
+
+ /* Do we have a color specification? */
+ if (!prefixcmp(sp, %C())
+ color_parse_mem(sp + 3,
+ ep - sp - 3,
+ --format , color);
+ else {
+ printf(%s, color);
Is it not possible for color to be used uninitialized here ?
+ print_value(info, parse_atom(sp + 2, ep),
quote_style);
+ }
}
if (*cp) {
sp = cp + strlen(cp);
--
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] Geolocation support
On Thu, May 23, 2013 at 10:45 AM, Ramkumar Ramachandra artag...@gmail.com wrote: Alessandro Di Marco wrote: this is a hack I made a couple of years ago in order to store my current location in git commits (I travel a lot and being able to associate a place with the commit date helps me to quickly recover what were doing at that time). Long story short, the screeenshot at http://tinypic.com/r/wars40/5 shows the new gitk interface once this patch has been integrated. Geolocation is controlled by two envvars GIT_AUTHOR_PLACE and COMMITTER_PLACE, respectively. You can set them via something like this: Obviously very interesting. Now, how do we mainline (parts of) this feature? I'll raise some questions here: I'm really not convinced this kind of changes should make it into Junio's tree (of course, he's the only one to decide). I really believe this is a very specific solution to a very specific problem (that is not for me to judge if the problem is real). Bloating the commit object with this kind of information doesn't feel like a good idea. I think it could be nice to provide a simple shell script to build the location, callable from a post-commit hook, to construct a geolocation note. Gitk could be programmed to read the notes to get the location, but once again, I'm not sure it should be mainlined. -- 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: [RFC/PATCH 2/3] pull: trivial cleanups
On Thu, May 16, 2013 at 11:36 AM, Felipe Contreras felipe.contre...@gmail.com wrote: Actually trying that command: % git fetch origin 'refs/tags/*:refs/tags/*' 'refs/heads/*:refs/heads/*' fatal: Refusing to fetch into current branch refs/heads/fc/fast-export/cleanup of non-bare repository fatal: The remote end hung up unexpectedly Can you try something like this instead ? git fetch --update-head-ok # or -u -- 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: is this a bug of git-diff?
On Wed, May 15, 2013 at 8:23 AM, eric liou accw...@gmail.com wrote: The output of git-diff is different from my expectation. It may skip some lines of context. git-diff is using a default of 3 lines of context above and below the changes. In your example, there is only two lines of context below the change, so only two lines are displayed. Above the change, three lines are displayed, as expected. That's why the blank line and leading slash line are not displayed. You can change the number of context lines by invoking git-diff with -Un. Hope that helps, Antoine -- 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: git grep --no-index doesn't do what it says on the tin?
On Wed, May 15, 2013 at 7:32 AM, Nazri Ramliy ayieh...@gmail.com wrote: Hi, From git help grep: --no-index Search files in the current directory that is not managed by Git. --untracked In addition to searching in the tracked files in the working tree, search also in untracked files. The difference is in the not managed by Git. Inside a repository, they will do the same thing. But only the first can work outside a repository. Cheers, Antoine -- 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: is this a bug of git-diff?
On Wed, May 15, 2013 at 8:52 AM, eric liou accw...@gmail.com wrote:
Thank you for the quick reply.
But this line is not correct: @@ -4,5 +4,6 @@ int a = 1;
Oh OK, I see.
Git tries to name the function where the changes take place. This is
purely informative.
In your example, you don't have any function so of course the
information is not very helpful.
Typically it will look like the following, helping the reader by
giving the function name:
@@ -591,6 +609,14 @@ int cmd_grep(int argc, const char **argv, const
char *prefix)
paths[1] = NULL;
}
+ if (!use_index) {
+ if (cached)
+ die(--cached cannot be used with --no-index.);
+ if (list.nr)
+ die(--no-index cannot be used with revs.);
+ return !grep_directory(opt, paths);
+ }
+
if (!list.nr) {
if (!cached)
setup_work_tree();
--
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: Premerging topics
On Wed, Apr 24, 2013 at 7:48 AM, Junio C Hamano gits...@pobox.com wrote: there could be textual conflicts and you could choose to leave them in, or you could choose to have rerere resolve it. As long as you do the same when replaying this prepackaged evil merge, this choice does not matter, but using rerere will make your life easier The problem is that rerere can not be easily shared, and I'm afraid it would be almost impossible to go without it (do you commit N with conflict markers hoping that F' will remove them?). But, as it looks like you would save F on top of M, it means that M would be reachable, and thus rerere would be recomputable from somewhere else. -- 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: Premerging topics (was: [RFD] annnotating a pair of commit objects?)
On Tue, Apr 23, 2013 at 8:34 AM, Johan Herland jo...@herland.net wrote:
On Wed, Apr 10, 2013 at 10:35 PM, Antoine Pelisse apeli...@gmail.com wrote:
Data-structure
==
We could use a note ref to store the pre-merge information. Each commit
would be annotated with a blob containing the list of pre-merges (one
sha1 per line with sha1 pointing to a merge commit). The commit on the
other side of a merge would also be annotated.
The choice of the refname could be done like we do with notes:
- Have a default value
- Have a default value configured in config
- Use a specific value when merging/creating the pre-merges
[snipped]
Cons
4. Checking connectivity means opening the blob and parsing it
Can you solve this problem with a tree object, instead of inventing a
specially-formatted blob?
That looks like a good idea.
I.e. given pre-merge info P for a merge between commits A and B: A is
annotated by a tree object that contains all pre-merges where A is
involved. Each entry in the tree object has a filename and a blob
SHA1; we store other commit involved in this pre-merge (B) in the
filename, and the pre-merge data (P) in the blob SHA1.
But P is a commit(/merge with two parents), not a blob. Can we have trees
pointing to commits instead of blobs ?
A nice side-effect of using tree objects to store pre-merge entries,
is that you can do a tree-level merge of them, and it'll do the Right
Thing (assuming two-way merge with no common history), i.e. perform a
union merge, and leave you to handle conflicts of individual
pre-merges (i.e. you'll only get conflicts when both sides offer
different pre-merge data for A + B).
I'm definitely not sure what you mean here, especially with tree-level
merge. Also I think we could be doing three-way merges. But I'm
no merge-strategies expert, so I'm kind of confused.
5. Regular notes and pre-merge notes have to be handled separately
because of 4.
Sort of, but you get that for any automated usage of notes for a
specific purpose. Just look at the notes-cache mechanism in
notes-cache.{h,c}. That's another example of functionality layered on
top of notes that makes assumptions on how its notes trees are
structured.
Thanks, I will have a look at it.
The goal is to merge branches J and B using existing pre-merges.
E0. Create an empty stack S
E1. Create set of commits 'J..B' and 'B..J' (that is probably already done)
E2. For each commit C in smallest(J..B, B..J), execute E3
E3. For each premerge P in notes-premerge(C), execute E4
E4. If one of both parents of P belongs to biggest(J..B, B..J), stack P in S
I don't think _both_ parents of P can belong to biggest(J..B, B..J).
AFAICS J..B and B..J must always be completely disjoint sets of
commits (this is easier to see when you consider that A..B is
equivalent to B ^A for any commits A and B), and in E2/E3, you have
already made sure that P has a parent in one of them. There is then no
way that the same parent can occur in the other set, so you have at
most _one_ parent in the other set.
I agree with that. After step E3, one of the parent belongs to one of
the two branches. Step E4 makes sure the other parent belongs to the other
branch (and not another unrelated branch).
E5. Merge J and B using all pre-merges from S
This is where things get complicated... :)
First there is one important thing that I have not seen a decision on
yet (maybe this was discussed in an earlier thread?):
Given pre-merge data P for commit A and B, does P encode the merge of
the entire history up to A with the entire history up to B, or does it
only encode the merging of the changes introduced in A with the
changes introduced in B? In other words, are we merging snapshots or
diffs?
In the former case, we only need to find the most recent commits A and
B on their respective branches - for which P exists - and then execute
that one P (or at most two Ps, if there is a criss-cross pre-merge
situation). In the other case, however, we need to enumerate all Ps
that apply to the two branches, and find a way to execute them
chronologically, dealing with missing Ps and conflicting Ps along the
way. IMHO, only the former approach seems practically solvable.
So you do not need to enumerate all Ps in J..B vs. B..J, you only need
to find the _most_ _recent_ P, and execute that one.
Indeed, we only need to know the most recent. That's a good point.
B1 B2B3
O---o---o-o
| / \ \
|P1 P2 M
|/| /
\__o_o___o
J1 J2 J3
In this use-case, we can use P1 to compute P2, and then we only need P2
to compute the real merge M. The idea would be to use P1 as an implicit
third parent to the pre-merge P2, and then P2 as an implicit third
parent to the real merge M.
$ git pre-merge topicA topicB topicC
to find, resolve and store all interactions between the topics.
Let's leave out octopus merges for now, and only
Re: Premerging topics
On Tue, Apr 23, 2013 at 5:29 PM, Junio C Hamano gits...@pobox.com wrote: Antoine Pelisse apeli...@gmail.com writes: And I have the feeling that merge-fix/B or merge-fix/A doesn't hold enough information to do that accurately. Oh, you do not have to resort to feeling; these names do _not_ hold enough information, period. We already know that, that was why I was unhappy, and that was why I sent the annotating a pair of commit objects RFD in the first place ;-). :) The idea is then to store the A, B pair as a note, and to associate a merge to that (solving the semantic conflict). OK, and as the datastore for A, B pair you were thinking about using a specially-formatted blob and Johan suggested to use a regular tree object? Exactly. But as I said, it should associate the pair to a merge. And trees contain other trees or blobs, not commits. I'm wondering if this is a problem. -- 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: Premerging topics (was: [RFD] annnotating a pair of commit objects?)
Any comment on that ? I think anyone using a Topic Workflow could use that feature and that it would be a nice addition to the project. Maybe I'm totally wrong in the proposal below (please tell me !), but there are some unanswered question that prevents me from starting (and I'd really like this to be discussed before actually starting). On Wed, Apr 10, 2013 at 10:35 PM, Antoine Pelisse apeli...@gmail.com wrote: The goal is to propose a structure for storing and pre-merging pairs of commits. Data-structure == We could use a note ref to store the pre-merge information. Each commit would be annotated with a blob containing the list of pre-merges (one sha1 per line with sha1 pointing to a merge commit). The commit on the other side of a merge would also be annotated. The choice of the refname could be done like we do with notes: - Have a default value - Have a default value configured in config - Use a specific value when merging/creating the pre-merges Here are my concerns: Pros 1. Notes allow dynamic annotation a commit 2. If we manage to fix 4, we can easily download all pre-merges from a remote host by fetching the ref (or clean by deleting the ref). 3. Conflicts on pre-merge notes could probably be resolved by concatenation. Cons 4. Checking connectivity means opening the blob and parsing it 5. Regular notes and pre-merge notes have to be handled separately because of 4. I'm hoping we can keep the pros and avoid the cons, but I'm kind of stuck here. Help would be really appreciated (or maybe this is a totally wrong direction, and I would also like to know ;) Merging (Using what we saved) = The goal is to merge branches J and B using existing pre-merges. E0. Create an empty stack S E1. Create set of commits 'J..B' and 'B..J' (that is probably already done) E2. For each commit C in smallest(J..B, B..J), execute E3 E3. For each premerge P in notes-premerge(C), execute E4 E4. If one of both parents of P belongs to biggest(J..B, B..J), stack P in S E5. Merge J and B using all pre-merges from S Let's consider that |J..B| is smaller than |B..J|. E0 is executed only once E1 is O(|J..B| + |B..J|) E2 is O(|J..B|) E3 is O(|J..B| x the average number of pre-merge per commits P_avg) E4 is executed for each parent (let's say it's two/constant, after all the topic is pair of commits), so still O(|J..B| x P_avg) E5 I don't know (how it can be done, and what would be the resulting time complexity) So the time cost for steps E0 to E4 is O(|J..B| + |B..J| x P_avg) Tools (Save the pre-merges) === Of course we need several tools to maintain the list of premerges, and to easily compute them. For example, it would be nice to be able to do something like: $ git pre-merge topicA topicB topicC to find, resolve and store all interactions between the topics. We could then easily derive to something that would allow to pre-merge a new topic with all topics already merged in master..pu (for example). Anyway, this task is left for latter. -- 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: git log - crash and core dump
On Tue, Apr 16, 2013 at 9:45 PM, Junio C Hamano gits...@pobox.com wrote: It still is curious how a malformed line was created in the first place. I wouldn't worry if a private tool used hash-object to create such a commit, but if it is something that is commonly used (e.g. git commit), others may suffer from the same and the tool needs to be tightened a bit. I already happened to see one like that, and it was clearly imported through remote-hg. I've not been able to reproduce though, and the parser in git-fast-import seemed already robust enough to me to not allow this kind of messed-up line. I will see if I can find some time to reproduce/investigate this deeper. -- 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] strbuf: create strbuf_humanize() to show byte sizes
Currently, humanization of downloaded size is done in the same
function as text formatting in 'process.c'. This is an issue if anyone
else wants to use this.
Separate text formatting from size simplification and make the function
public in strbuf so that it can easily be used by other clients.
We now can use strbuf_humanize() for both downloaded size and download
speed calculation. One of the drawbacks is that speed will now look like
this when download is stalled: 0 bytes/s instead of 0 KiB/s.
Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
Documentation/technical/api-strbuf.txt |5
progress.c | 43 +++-
strbuf.c | 19 ++
strbuf.h |1 +
4 files changed, 40 insertions(+), 28 deletions(-)
diff --git a/Documentation/technical/api-strbuf.txt
b/Documentation/technical/api-strbuf.txt
index 2c59cb2..7b6ecda 100644
--- a/Documentation/technical/api-strbuf.txt
+++ b/Documentation/technical/api-strbuf.txt
@@ -230,6 +230,11 @@ which can be used by the programmer of the callback as she
sees fit.
destination. This is useful for literal data to be fed to either
strbuf_expand or to the *printf family of functions.
+`strbuf_humanize`::
+
+ Append the given byte size as a human-readable string (i.e. 12.23 KiB,
+ 3.50 MiB).
+
`strbuf_addf`::
Add a formatted string to the buffer.
diff --git a/progress.c b/progress.c
index 3971f49..8e09058 100644
--- a/progress.c
+++ b/progress.c
@@ -10,6 +10,7 @@
#include git-compat-util.h
#include progress.h
+#include strbuf.h
#define TP_IDX_MAX 8
@@ -112,34 +113,14 @@ static int display(struct progress *progress, unsigned n,
const char *done)
return 0;
}
-static void throughput_string(struct throughput *tp, off_t total,
+static void throughput_string(struct strbuf *buf, off_t total,
unsigned int rate)
{
- int l = sizeof(tp-display);
- if (total 1 30) {
- l -= snprintf(tp-display, l, , %u.%2.2u GiB,
- (int)(total 30),
- (int)(total ((1 30) - 1)) / 10737419);
- } else if (total 1 20) {
- int x = total + 5243; /* for rounding */
- l -= snprintf(tp-display, l, , %u.%2.2u MiB,
- x 20, ((x ((1 20) - 1)) * 100) 20);
- } else if (total 1 10) {
- int x = total + 5; /* for rounding */
- l -= snprintf(tp-display, l, , %u.%2.2u KiB,
- x 10, ((x ((1 10) - 1)) * 100) 10);
- } else {
- l -= snprintf(tp-display, l, , %u bytes, (int)total);
- }
-
- if (rate 1 10) {
- int x = rate + 5; /* for rounding */
- snprintf(tp-display + sizeof(tp-display) - l, l,
- | %u.%2.2u MiB/s,
-x 10, ((x ((1 10) - 1)) * 100) 10);
- } else if (rate)
- snprintf(tp-display + sizeof(tp-display) - l, l,
- | %u KiB/s, rate);
+ strbuf_addstr(buf, , );
+ strbuf_humanize(buf, total);
+ strbuf_addstr(buf, | );
+ strbuf_humanize(buf, rate * 1024);
+ strbuf_addstr(buf, /s);
}
void display_throughput(struct progress *progress, off_t total)
@@ -183,6 +164,7 @@ void display_throughput(struct progress *progress, off_t
total)
misecs += (int)(tv.tv_usec - tp-prev_tv.tv_usec) / 977;
if (misecs 512) {
+ struct strbuf buf = STRBUF_INIT;
unsigned int count, rate;
count = total - tp-prev_total;
@@ -197,7 +179,9 @@ void display_throughput(struct progress *progress, off_t
total)
tp-last_misecs[tp-idx] = misecs;
tp-idx = (tp-idx + 1) % TP_IDX_MAX;
- throughput_string(tp, total, rate);
+ throughput_string(buf, total, rate);
+ strncpy(tp-display, buf.buf, sizeof(tp-display));
+ strbuf_release(buf);
if (progress-last_value != -1 progress_update)
display(progress, progress-last_value, NULL);
}
@@ -253,9 +237,12 @@ void stop_progress_msg(struct progress **p_progress, const
char *msg)
bufp = (len sizeof(buf)) ? buf : xmalloc(len + 1);
if (tp) {
+ struct strbuf strbuf = STRBUF_INIT;
unsigned int rate = !tp-avg_misecs ? 0 :
tp-avg_bytes / tp-avg_misecs;
- throughput_string(tp, tp-curr_total, rate);
+ throughput_string(strbuf, tp-curr_total, rate);
+ strncpy(tp-display, strbuf.buf, sizeof(tp-display));
+ strbuf_release(strbuf);
}
progress_update = 1
[PATCH 2/2] count-objects: add -H option to humanize sizes
Use the new humanize() function to print loose objects size, pack size,
and garbage size in verbose mode, or loose objects size in regular mode.
This patch doesn't change the way anything is displayed when the option
is not used.
Also update the documentation.
Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
Documentation/git-count-objects.txt | 14 ---
builtin/count-objects.c | 46 +--
2 files changed, 48 insertions(+), 12 deletions(-)
diff --git a/Documentation/git-count-objects.txt
b/Documentation/git-count-objects.txt
index da6e72e..b300e84 100644
--- a/Documentation/git-count-objects.txt
+++ b/Documentation/git-count-objects.txt
@@ -8,7 +8,7 @@ git-count-objects - Count unpacked number of objects and their
disk consumption
SYNOPSIS
[verse]
-'git count-objects' [-v]
+'git count-objects' [-v] [-H | --human-readable]
DESCRIPTION
---
@@ -24,11 +24,11 @@ OPTIONS
+
count: the number of loose objects
+
-size: disk space consumed by loose objects, in KiB
+size: disk space consumed by loose objects, in KiB (unless -H is specified)
+
in-pack: the number of in-pack objects
+
-size-pack: disk space consumed by the packs, in KiB
+size-pack: disk space consumed by the packs, in KiB (unless -H is specified)
+
prune-packable: the number of loose objects that are also present in
the packs. These objects could be pruned using `git prune-packed`.
@@ -36,7 +36,13 @@ the packs. These objects could be pruned using `git
prune-packed`.
garbage: the number of files in object database that are not valid
loose objects nor valid packs
+
-size-garbage: disk space consumed by garbage files, in KiB
+size-garbage: disk space consumed by garbage files, in KiB (unless -H is
+specified)
+
+-H::
+--human-readable::
+
+Print sizes in human readable format
GIT
---
diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index 0343e35..935ad9e 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -79,13 +79,13 @@ static void count_objects(DIR *d, char *path, int len, int
verbose,
}
static char const * const count_objects_usage[] = {
- N_(git count-objects [-v]),
+ N_(git count-objects [-v] [-H | --human-readable]),
NULL
};
int cmd_count_objects(int argc, const char **argv, const char *prefix)
{
- int i, verbose = 0;
+ int i, verbose = 0, human_readable = 0;
const char *objdir = get_object_directory();
int len = strlen(objdir);
char *path = xmalloc(len + 50);
@@ -93,6 +93,8 @@ int cmd_count_objects(int argc, const char **argv, const char
*prefix)
off_t loose_size = 0;
struct option opts[] = {
OPT__VERBOSE(verbose, N_(be verbose)),
+ OPT_BOOLEAN('H', human-readable, human_readable,
+ N_(print sizes in human readable format)),
OPT_END(),
};
@@ -119,6 +121,9 @@ int cmd_count_objects(int argc, const char **argv, const
char *prefix)
struct packed_git *p;
unsigned long num_pack = 0;
off_t size_pack = 0;
+ struct strbuf loose_buf = STRBUF_INIT;
+ struct strbuf pack_buf = STRBUF_INIT;
+ struct strbuf garbage_buf = STRBUF_INIT;
if (!packed_git)
prepare_packed_git();
for (p = packed_git; p; p = p-next) {
@@ -130,17 +135,42 @@ int cmd_count_objects(int argc, const char **argv, const
char *prefix)
size_pack += p-pack_size + p-index_size;
num_pack++;
}
+
+ if (human_readable) {
+ strbuf_humanize(loose_buf, loose_size);
+ strbuf_humanize(pack_buf, size_pack);
+ strbuf_humanize(garbage_buf, size_garbage);
+ }
+ else {
+ strbuf_addf(loose_buf, %lu,
+ (unsigned long)(loose_size / 1024));
+ strbuf_addf(pack_buf, %lu,
+ (unsigned long)(size_pack / 1024));
+ strbuf_addf(garbage_buf, %lu,
+ (unsigned long)(size_garbage / 1024));
+ }
+
printf(count: %lu\n, loose);
- printf(size: %lu\n, (unsigned long) (loose_size / 1024));
+ printf(size: %s\n, loose_buf.buf);
printf(in-pack: %lu\n, packed);
printf(packs: %lu\n, num_pack);
- printf(size-pack: %lu\n, (unsigned long) (size_pack / 1024));
+ printf(size-pack: %s\n, pack_buf.buf);
printf(prune-packable: %lu\n, packed_loose);
printf(garbage: %lu\n, garbage);
- printf(size-garbage: %lu\n, (unsigned long) (size_garbage /
1024));
+ printf(size-garbage: %s\n, garbage_buf.buf
Re: [PATCH 1/2] strbuf: create strbuf_humanize() to show byte sizes
On Wed, Apr 10, 2013 at 9:43 PM, Jonathan Nieder jrnie...@gmail.com wrote: Antoine Pelisse wrote: One of the drawbacks is that speed will now look like this when download is stalled: 0 bytes/s instead of 0 KiB/s. At first glance that is neither obviously a benefit nor obviously a drawback. Can you spell this out more? The drawback to me is that it changes the user experience with no reason. But that's really a minor change, I agree. (maybe I should have put it as a comment/question after ---) --- a/Documentation/technical/api-strbuf.txt +++ b/Documentation/technical/api-strbuf.txt @@ -230,6 +230,11 @@ which can be used by the programmer of the callback as she sees fit. destination. This is useful for literal data to be fed to either strbuf_expand or to the *printf family of functions. +`strbuf_humanize`:: + + Append the given byte size as a human-readable string (i.e. 12.23 KiB, + 3.50 MiB). Based on the function name alone, it is not easy to guess what it will do (e.g., maybe it will paraphrase 3 to three and 1000 to enormous). How about something like strbuf_filesize? I think the suggestion from Junio makes more sense, as it can be used for download speed. If I understand the code correctly, this jumps units each time it exceeds 1.0 of the next unit (bytes, KiB, MiB, GiB), which sounds like a fine behavior. The code has simply been extracted from the former function and kept unmodified. Thanks for the help ! Antoine, -- 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 1/2] strbuf: create strbuf_humanize() to show byte sizes
On Wed, Apr 10, 2013 at 9:57 PM, Junio C Hamano gits...@pobox.com wrote: Antoine Pelisse apeli...@gmail.com writes: Currently, humanization of downloaded size is done in the same function as text formatting in 'process.c'. This is an issue if anyone else wants to use this. Separate text formatting from size simplification and make the function public in strbuf so that it can easily be used by other clients. We now can use strbuf_humanize() for both downloaded size and download speed calculation. One of the drawbacks is that speed will now look like this when download is stalled: 0 bytes/s instead of 0 KiB/s. Personally, I do not think the drawback is so big an issue. If the caller really cares, we could always add another parameter to this formatter to tell it the minimum unit we care about (e.g. pass 1024 to say Don't bother showing scale lower than kibi). I thought about that, but decided it was not worth it (at least for the moment) This is a bit late response, but if we ever want to count something in a dimention other than bytes, like time (e.g. kiloseconds) or number of commits (e.g. centicommits), etc., we cannot reuse this formatter very easily. We may want to have byte somewhere in its name for now to make sure the callers understand its limitation. I'm not in a hurry. But it look tough to make it generic: one is binary, another is sexagesimal, and the last is decimal I'll tentatively rename it to strbuf_humanize_bytes() while queuing. I like the idea, 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
Premerging topics (was: [RFD] annnotating a pair of commit objects?)
The goal is to propose a structure for storing and pre-merging pairs of commits. Data-structure == We could use a note ref to store the pre-merge information. Each commit would be annotated with a blob containing the list of pre-merges (one sha1 per line with sha1 pointing to a merge commit). The commit on the other side of a merge would also be annotated. The choice of the refname could be done like we do with notes: - Have a default value - Have a default value configured in config - Use a specific value when merging/creating the pre-merges Here are my concerns: Pros 1. Notes allow dynamic annotation a commit 2. If we manage to fix 4, we can easily download all pre-merges from a remote host by fetching the ref (or clean by deleting the ref). 3. Conflicts on pre-merge notes could probably be resolved by concatenation. Cons 4. Checking connectivity means opening the blob and parsing it 5. Regular notes and pre-merge notes have to be handled separately because of 4. I'm hoping we can keep the pros and avoid the cons, but I'm kind of stuck here. Help would be really appreciated (or maybe this is a totally wrong direction, and I would also like to know ;) Merging (Using what we saved) = The goal is to merge branches J and B using existing pre-merges. E0. Create an empty stack S E1. Create set of commits 'J..B' and 'B..J' (that is probably already done) E2. For each commit C in smallest(J..B, B..J), execute E3 E3. For each premerge P in notes-premerge(C), execute E4 E4. If one of both parents of P belongs to biggest(J..B, B..J), stack P in S E5. Merge J and B using all pre-merges from S Let's consider that |J..B| is smaller than |B..J|. E0 is executed only once E1 is O(|J..B| + |B..J|) E2 is O(|J..B|) E3 is O(|J..B| x the average number of pre-merge per commits P_avg) E4 is executed for each parent (let's say it's two/constant, after all the topic is pair of commits), so still O(|J..B| x P_avg) E5 I don't know (how it can be done, and what would be the resulting time complexity) So the time cost for steps E0 to E4 is O(|J..B| + |B..J| x P_avg) Tools (Save the pre-merges) === Of course we need several tools to maintain the list of premerges, and to easily compute them. For example, it would be nice to be able to do something like: $ git pre-merge topicA topicB topicC to find, resolve and store all interactions between the topics. We could then easily derive to something that would allow to pre-merge a new topic with all topics already merged in master..pu (for example). Anyway, this task is left for latter. -- 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] progress: create public humanize() to show sizes
Currently, humanization of downloaded size is done in the same function
as text formatting. This is an issue if anyone else wants to use this.
Separate text formatting from size simplification and make the function
public so that it can easily be used by other clients.
We now can use humanize() for both downloaded size and download speed
calculation. One of the drawbacks is that speed will no look like this
when download is stalled: 0 bytes/s instead of 0 KiB/s.
Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
progress.c | 60 ++--
progress.h |2 ++
2 files changed, 36 insertions(+), 26 deletions(-)
diff --git a/progress.c b/progress.c
index 3971f49..76c1e42 100644
--- a/progress.c
+++ b/progress.c
@@ -8,8 +8,11 @@
* published by the Free Software Foundation.
*/
+#include string.h
+
#include git-compat-util.h
#include progress.h
+#include strbuf.h
#define TP_IDX_MAX 8
@@ -112,34 +115,33 @@ static int display(struct progress *progress, unsigned n,
const char *done)
return 0;
}
-static void throughput_string(struct throughput *tp, off_t total,
- unsigned int rate)
+void humanize(struct strbuf *buf, off_t bytes)
{
- int l = sizeof(tp-display);
- if (total 1 30) {
- l -= snprintf(tp-display, l, , %u.%2.2u GiB,
- (int)(total 30),
- (int)(total ((1 30) - 1)) / 10737419);
- } else if (total 1 20) {
- int x = total + 5243; /* for rounding */
- l -= snprintf(tp-display, l, , %u.%2.2u MiB,
- x 20, ((x ((1 20) - 1)) * 100) 20);
- } else if (total 1 10) {
- int x = total + 5; /* for rounding */
- l -= snprintf(tp-display, l, , %u.%2.2u KiB,
- x 10, ((x ((1 10) - 1)) * 100) 10);
+ if (bytes 1 30) {
+ strbuf_addf(buf, %u.%2.2u GiB,
+ (int)(bytes 30),
+ (int)(bytes ((1 30) - 1)) / 10737419);
+ } else if (bytes 1 20) {
+ int x = bytes + 5243; /* for rounding */
+ strbuf_addf(buf, %u.%2.2u MiB,
+ x 20, ((x ((1 20) - 1)) * 100) 20);
+ } else if (bytes 1 10) {
+ int x = bytes + 5; /* for rounding */
+ strbuf_addf(buf, %u.%2.2u KiB,
+ x 10, ((x ((1 10) - 1)) * 100) 10);
} else {
- l -= snprintf(tp-display, l, , %u bytes, (int)total);
+ strbuf_addf(buf, %u bytes, (int)bytes);
}
+}
- if (rate 1 10) {
- int x = rate + 5; /* for rounding */
- snprintf(tp-display + sizeof(tp-display) - l, l,
- | %u.%2.2u MiB/s,
-x 10, ((x ((1 10) - 1)) * 100) 10);
- } else if (rate)
- snprintf(tp-display + sizeof(tp-display) - l, l,
- | %u KiB/s, rate);
+static void throughput_string(struct strbuf *buf, off_t total,
+ unsigned int rate)
+{
+ strbuf_addstr(buf, , );
+ humanize(buf, total);
+ strbuf_addstr(buf, | );
+ humanize(buf, rate * 1024);
+ strbuf_addstr(buf, /s);
}
void display_throughput(struct progress *progress, off_t total)
@@ -183,6 +185,7 @@ void display_throughput(struct progress *progress, off_t
total)
misecs += (int)(tv.tv_usec - tp-prev_tv.tv_usec) / 977;
if (misecs 512) {
+ struct strbuf buf = STRBUF_INIT;
unsigned int count, rate;
count = total - tp-prev_total;
@@ -197,7 +200,9 @@ void display_throughput(struct progress *progress, off_t
total)
tp-last_misecs[tp-idx] = misecs;
tp-idx = (tp-idx + 1) % TP_IDX_MAX;
- throughput_string(tp, total, rate);
+ throughput_string(buf, total, rate);
+ strncpy(tp-display, buf.buf, sizeof(tp-display));
+ strbuf_release(buf);
if (progress-last_value != -1 progress_update)
display(progress, progress-last_value, NULL);
}
@@ -253,9 +258,12 @@ void stop_progress_msg(struct progress **p_progress, const
char *msg)
bufp = (len sizeof(buf)) ? buf : xmalloc(len + 1);
if (tp) {
+ struct strbuf strbuf = STRBUF_INIT;
unsigned int rate = !tp-avg_misecs ? 0 :
tp-avg_bytes / tp-avg_misecs;
- throughput_string(tp, tp-curr_total, rate);
+ throughput_string(strbuf, tp-curr_total, rate);
+ strncpy(tp-display, strbuf.buf, sizeof(tp-display));
+ strbuf_release(strbuf
[PATCH 2/2] count-objects: add -H option to humanize sizes
Use the new humanize() function to print loose objects size, pack size,
and garbage size in verbose mode, or loose objects size in regular mode.
This patch doesn't change the way anything is displayed when the option
is not used.
Also update the documentation.
Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
Documentation/git-count-objects.txt | 14 ---
builtin/count-objects.c | 47 +--
2 files changed, 49 insertions(+), 12 deletions(-)
diff --git a/Documentation/git-count-objects.txt
b/Documentation/git-count-objects.txt
index da6e72e..b300e84 100644
--- a/Documentation/git-count-objects.txt
+++ b/Documentation/git-count-objects.txt
@@ -8,7 +8,7 @@ git-count-objects - Count unpacked number of objects and their
disk consumption
SYNOPSIS
[verse]
-'git count-objects' [-v]
+'git count-objects' [-v] [-H | --human-readable]
DESCRIPTION
---
@@ -24,11 +24,11 @@ OPTIONS
+
count: the number of loose objects
+
-size: disk space consumed by loose objects, in KiB
+size: disk space consumed by loose objects, in KiB (unless -H is specified)
+
in-pack: the number of in-pack objects
+
-size-pack: disk space consumed by the packs, in KiB
+size-pack: disk space consumed by the packs, in KiB (unless -H is specified)
+
prune-packable: the number of loose objects that are also present in
the packs. These objects could be pruned using `git prune-packed`.
@@ -36,7 +36,13 @@ the packs. These objects could be pruned using `git
prune-packed`.
garbage: the number of files in object database that are not valid
loose objects nor valid packs
+
-size-garbage: disk space consumed by garbage files, in KiB
+size-garbage: disk space consumed by garbage files, in KiB (unless -H is
+specified)
+
+-H::
+--human-readable::
+
+Print sizes in human readable format
GIT
---
diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index 0343e35..9836f6a 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -8,6 +8,7 @@
#include dir.h
#include builtin.h
#include parse-options.h
+#include progress.h
static unsigned long garbage;
static off_t size_garbage;
@@ -79,13 +80,13 @@ static void count_objects(DIR *d, char *path, int len, int
verbose,
}
static char const * const count_objects_usage[] = {
- N_(git count-objects [-v]),
+ N_(git count-objects [-v] [-H | --human-readable]),
NULL
};
int cmd_count_objects(int argc, const char **argv, const char *prefix)
{
- int i, verbose = 0;
+ int i, verbose = 0, human_readable = 0;
const char *objdir = get_object_directory();
int len = strlen(objdir);
char *path = xmalloc(len + 50);
@@ -93,6 +94,8 @@ int cmd_count_objects(int argc, const char **argv, const char
*prefix)
off_t loose_size = 0;
struct option opts[] = {
OPT__VERBOSE(verbose, N_(be verbose)),
+ OPT_BOOLEAN('H', human-readable, human_readable,
+ N_(print sizes in human readable format)),
OPT_END(),
};
@@ -119,6 +122,9 @@ int cmd_count_objects(int argc, const char **argv, const
char *prefix)
struct packed_git *p;
unsigned long num_pack = 0;
off_t size_pack = 0;
+ struct strbuf loose_buf = STRBUF_INIT;
+ struct strbuf pack_buf = STRBUF_INIT;
+ struct strbuf garbage_buf = STRBUF_INIT;
if (!packed_git)
prepare_packed_git();
for (p = packed_git; p; p = p-next) {
@@ -130,17 +136,42 @@ int cmd_count_objects(int argc, const char **argv, const
char *prefix)
size_pack += p-pack_size + p-index_size;
num_pack++;
}
+
+ if (human_readable) {
+ humanize(loose_buf, loose_size);
+ humanize(pack_buf, size_pack);
+ humanize(garbage_buf, size_garbage);
+ }
+ else {
+ strbuf_addf(loose_buf, %lu,
+ (unsigned long)(loose_size / 1024));
+ strbuf_addf(pack_buf, %lu,
+ (unsigned long)(size_pack / 1024));
+ strbuf_addf(garbage_buf, %lu,
+ (unsigned long)(size_garbage / 1024));
+ }
+
printf(count: %lu\n, loose);
- printf(size: %lu\n, (unsigned long) (loose_size / 1024));
+ printf(size: %s\n, loose_buf.buf);
printf(in-pack: %lu\n, packed);
printf(packs: %lu\n, num_pack);
- printf(size-pack: %lu\n, (unsigned long) (size_pack / 1024));
+ printf(size-pack: %s\n, pack_buf.buf);
printf(prune-packable: %lu\n, packed_loose);
printf(garbage: %lu\n, garbage
[PATCH] remote-helpers: remove --graph in hg_log()
The hg_log() test helper uses the --graph parameter that is
implemented by the GraphLog extension. If the extension is not activated
by the user, the parameter is not available. Do not use the option that
is unnecessary.
Also changes the way we grep the output in hg_log(). The pipe operator
can hide the return code of hg command. As a matter of fact, if log
fails because it doesn't know about --graph, it doesn't report any
failure and let's you think everything worked.
Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
Hey Felipe,
I'm not so confident that --graph is useless to the test. If it's really
necessary, it would be nice either to activate it in setup() or to use
it just for the command through: --config extensions.graphlog=.
Cheers,
contrib/remote-helpers/test-hg-bidi.sh |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/contrib/remote-helpers/test-hg-bidi.sh
b/contrib/remote-helpers/test-hg-bidi.sh
index 1d61982..96cd5a7 100755
--- a/contrib/remote-helpers/test-hg-bidi.sh
+++ b/contrib/remote-helpers/test-hg-bidi.sh
@@ -50,7 +50,8 @@ hg_push () {
}
hg_log () {
- hg -R $1 log --graph --debug | grep -v 'tag: *default/'
+ hg -R $1 log --debug log
+ grep -v 'tag: *default/' log
}
setup () {
--
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] remote-helpers: remove --graph in hg_log()
On Sat, Apr 6, 2013 at 6:06 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Sat, Apr 6, 2013 at 10:00 AM, Antoine Pelisse apeli...@gmail.com wrote: I'm not so confident that --graph is useless to the test. If it's really necessary, it would be nice either to activate it in setup() or to use it just for the command through: --config extensions.graphlog=. I think it should be activated in the setup, it comes packaged with mercurial, and it's likely that many users have it enabled. But is it relevant to the tests ? I have the feeling that it's not strictly necessary to both add an extension to hgrc and a command line option. (and indeed, the tests still work for me, but maybe I'm missing something). -- 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] remote-hg: activate graplog extension for hg_log()
The hg_log() test helper uses the --graph parameter that is
implemented by the GraphLog extension. If the extension is not activated
by the user, the parameter is not available. Activate the extension in
setup().
Also changes the way we grep the output in hg_log(). The pipe operator
can hide the return code of hg command. As a matter of fact, if log
fails because it doesn't know about --graph, it doesn't report any
failure and let's you think everything worked.
Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
- Updated the title to use remote-hg instead of remote-helpers
- Activate the extension instead of removing the option
- Apply the same to test-hg-hg-git
contrib/remote-helpers/test-hg-bidi.sh |5 -
contrib/remote-helpers/test-hg-hg-git.sh |4 +++-
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/contrib/remote-helpers/test-hg-bidi.sh
b/contrib/remote-helpers/test-hg-bidi.sh
index 1d61982..84b9228 100755
--- a/contrib/remote-helpers/test-hg-bidi.sh
+++ b/contrib/remote-helpers/test-hg-bidi.sh
@@ -50,7 +50,8 @@ hg_push () {
}
hg_log () {
- hg -R $1 log --graph --debug | grep -v 'tag: *default/'
+ hg -R $1 log --graph --debug log
+ grep -v 'tag: *default/' log
}
setup () {
@@ -62,6 +63,8 @@ setup () {
echo commit = -d \0 0\
echo debugrawcommit = -d \0 0\
echo tag = -d \0 0\
+ echo [extensions]
+ echo graphlog =
) $HOME/.hgrc
git config --global remote-hg.hg-git-compat true
diff --git a/contrib/remote-helpers/test-hg-hg-git.sh
b/contrib/remote-helpers/test-hg-hg-git.sh
index 7e3967f..17a074e 100755
--- a/contrib/remote-helpers/test-hg-hg-git.sh
+++ b/contrib/remote-helpers/test-hg-hg-git.sh
@@ -78,7 +78,8 @@ hg_push_hg () {
}
hg_log () {
- hg -R $1 log --graph --debug | grep -v 'tag: *default/'
+ hg -R $1 log --graph --debug log
+ grep -v 'tag: *default/' log
}
git_log () {
@@ -97,6 +98,7 @@ setup () {
echo [extensions]
echo hgext.bookmarks =
echo hggit =
+ echo graphlog =
) $HOME/.hgrc
git config --global receive.denycurrentbranch warn
git config --global remote-hg.hg-git-compat true
--
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] fast-export: Allow pruned-references in mark file
fast-export can fail because of some pruned-reference when importing a
mark file.
The problem happens in the following scenario:
$ git fast-export --export-marks=MARKS master
(rewrite master)
$ git prune
$ git fast-export --import-marks=MARKS master
This might fail if some references have been removed by prune
because some marks will refer to no longer existing commits.
git-fast-export will not need these objects anyway as they were no
longer reachable.
We still need to update last_numid so we don't change the mapping
between marks and objects for remote-helpers.
Unfortunately, the mark file should not be rewritten without lost marks
if no new objects has been exported, as we could lose track of the last
last_numid.
Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
Documentation/git-fast-export.txt |2 ++
builtin/fast-export.c | 11 +++
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-fast-export.txt
b/Documentation/git-fast-export.txt
index d6487e1..feab7a3 100644
--- a/Documentation/git-fast-export.txt
+++ b/Documentation/git-fast-export.txt
@@ -66,6 +66,8 @@ produced incorrect results if you gave these options.
incremental runs. As file is only opened and truncated
at completion, the same path can also be safely given to
\--import-marks.
+ The file will not be written if no new object has been
+ marked/exported.
--import-marks=file::
Before processing any input, load the marks specified in
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index d380155..f44b76c 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -618,9 +618,12 @@ static void import_marks(char *input_file)
|| *mark_end != ' ' || get_sha1(mark_end + 1, sha1))
die(corrupt mark line: %s, line);
+ if (last_idnum mark)
+ last_idnum = mark;
+
object = parse_object(sha1);
if (!object)
- die (Could not read blob %s, sha1_to_hex(sha1));
+ continue;
if (object-flags SHOWN)
error(Object %s already has a mark,
sha1_to_hex(sha1));
@@ -630,8 +633,6 @@ static void import_marks(char *input_file)
continue;
mark_object(object, mark);
- if (last_idnum mark)
- last_idnum = mark;
object-flags |= SHOWN;
}
@@ -645,6 +646,7 @@ int cmd_fast_export(int argc, const char **argv, const char
*prefix)
struct string_list extra_refs = STRING_LIST_INIT_NODUP;
struct commit *commit;
char *export_filename = NULL, *import_filename = NULL;
+ uint32_t lastimportid;
struct option options[] = {
OPT_INTEGER(0, progress, progress,
N_(show progress after n objects)),
@@ -688,6 +690,7 @@ int cmd_fast_export(int argc, const char **argv, const char
*prefix)
if (import_filename)
import_marks(import_filename);
+ lastimportid = last_idnum;
if (import_filename revs.prune_data.nr)
full_tree = 1;
@@ -710,7 +713,7 @@ int cmd_fast_export(int argc, const char **argv, const char
*prefix)
handle_tags_and_duplicates(extra_refs);
- if (export_filename)
+ if (export_filename lastimportid != last_idnum)
export_marks(export_filename);
if (use_done_feature)
--
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: [RFD] annnotating a pair of commit objects?
On Thu, Jan 3, 2013 at 10:59 AM, Michael Haggerty mhag...@alum.mit.edu wrote: On 01/03/2013 08:03 AM, Junio C Hamano wrote: I'd like a datastore that maps a pair of commit object names to another object name, such that: * When looking at two commits A and B, efficiently query all data associated with a pair of commits X,Y where X is contained in the range A..B and not in B..A, and Y is contained in the range B..A and not in A..B. * When X,Y is registered in the datastore, and X is rewritten to X' and/or Y is rewritten to Y', the mapping is updated so that it can be queried with X',Y' as a new key, similar to the way a notes tree that maps object X can be updated to map object X' when such a rewrite happens. The intended use case is to go beyond rerere. Given a history of this shape: o---o---o---I mainline / O---o---X---o---Atopic A \ o---Y---o---o---B topic B I would indeed also be interested in such a feature for my day-to-day work where we use a workflow similar to git.git's. When doing this merge, I think your goal is equivalent to discovering that M includes part of the merge of J and B, and adding M as an (implicit or explicit) third parent to the merge: o---o---o---I---J---K mainline / /. / O---o---X---o---A. /topic A \ \ . / \ M. / \ / / o---Y---o---o---Btopic B How could M be stored? Assuming that these type of premerge merges are sparse, then Jeff's analysis seems good. Concretely, one could simply store pointers to M from both X and Y; e.g., * Add a note to X with the information when merging this commit with Y, use premerge M * Add a note to Y with the information when merging this commit with X, use premerge M Then, when merging, create the set J..B, scan all of the commits on B..J for these premerge notes (O(|B..J|)), and for each one, look in the set J..B to see if it is present. The effort should scale like O( |J..B| + |B..J| * lg(|J..B|) ) where, of course J and B could be exchanged for either aesthetic or performance reasons. (One would also need a mechanism for preventing M from being garbage-collected.) I like the idea of using notes and a kind of pre-merge. The proposal seems decent to me. Michael, have you started implementing such a thing ? If you did, I would love to help as much as I can. If you didn't, I would like to start implementing this feature (I think I now have some time to do so). Maybe that would require some kind of mentoring though. It could be a nice opportunity for the community to improve that too as a fake gsoc (no google, no summer, no student) -- 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 v2] count-objects: output KiB instead of kilobytes
Should we use that opportunity to implement an option like -h (for humanize) similar to what ls(1), df(1), du(1) does ? Of course -h is already used for help, so we could use -H or any other sensible choice. It can become tough to read the size when it gets big enough. On Thu, Apr 4, 2013 at 6:27 PM, Junio C Hamano gits...@pobox.com wrote: Mihai Capotă mi...@mihaic.ro writes: The git manual contains an explicit warning about the output of a porcelain command changing: The interface to Porcelain commands on the other hand are subject to change in order to improve the end user experience. Yeah, I know that, as I wrote it ;-) Aside from count-object being not exactly a Porcelain, the statement does not give us a blank check to make random changes as we see fit. There needs to be a clear improvement. I am just having a hard time weighing the benefit of using more accurate kibibytes over kilobytes and the possible downside of breaking other peoples' tools. Perhaps it would be alright if the change was accompanied by a warning in the Release Notes to say something like: If you have scripts that decide when to run git repack by parsing the output from git count-objects, this release may break them. Sorry about that. One of the scripts shipped by git-core itself also had to be adjusted. The command reports the total diskspace used to store loose objects in kibibytes, but it was labelled as kilobytes. The number now is shown with KiB, e.g. 6750 objects, 50928 KiB. You may want to consider updating such scripts to always call git gc --auto to let it decide when to repack for you. Also, I suspect that for the purpose of this exact output field, nobody cares the difference between kibibytes and kilobytes. Depending on the system, we add up either st.st_blocks or st.st_size and the result is not that exact as how much diskspace is consumed. -- 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 -- 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 00/13] remote-hg: general updates
* internally, the marks are using the hg sha1s instead of the hg rev ids. The latter are not necessarily invariant, and using the sha1s makes it much easier to recover from semi-broken states. I doubt this makes any difference (except for more wasted space). I think this is definitely wrong. If you happen to strip a changeset from the mercurial repository, and redo a completely different commit on top of it, the new commit will never be seen on git end (because it will have the same rev id and will thus be identified as identical from git point of view). -- 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] test-lint-duplicates: check numbering in contrib/remote-helpers
On Tue, Apr 2, 2013 at 6:53 PM, Torsten Bögershausen tbo...@web.de wrote: --- contrib/remote-helpers/Makefile| 3 +- contrib/remote-helpers/t5810-test-bzr.sh | 143 +++ contrib/remote-helpers/t5820-test-hg-bidi.sh | 243 +++ contrib/remote-helpers/t5821-test-hg-hg-git.sh | 534 + contrib/remote-helpers/t5830-test-hg.sh| 121 ++ contrib/remote-helpers/test-bzr.sh | 143 --- contrib/remote-helpers/test-hg-bidi.sh | 243 --- contrib/remote-helpers/test-hg-hg-git.sh | 534 - contrib/remote-helpers/test-hg.sh | 121 -- t/Makefile | 2 +- 10 files changed, 1044 insertions(+), 1043 deletions(-) create mode 100755 contrib/remote-helpers/t5810-test-bzr.sh create mode 100755 contrib/remote-helpers/t5820-test-hg-bidi.sh create mode 100755 contrib/remote-helpers/t5821-test-hg-hg-git.sh create mode 100755 contrib/remote-helpers/t5830-test-hg.sh delete mode 100755 contrib/remote-helpers/test-bzr.sh delete mode 100755 contrib/remote-helpers/test-hg-bidi.sh delete mode 100755 contrib/remote-helpers/test-hg-hg-git.sh delete mode 100755 contrib/remote-helpers/test-hg.sh Don't hesitate to format patches with -M :) -- 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 1/3] remote-helpers: fix the run of all tests
Hey Felipe, On Mon, Apr 1, 2013 at 11:14 PM, Felipe Contreras felipe.contre...@gmail.com wrote: +export TEST_LINT := I think test-lint-executable still makes sense here. -- 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 v9a 3/5] Implement line-history search (git log -L)
diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh
new file mode 100755
index 000..286390d
--- /dev/null
+++ b/t/t4211-line-log.sh
@@ -0,0 +1,38 @@
+#!/bin/sh
+
+test_description='test log -L'
+. ./test-lib.sh
+
+test_expect_success 'setup (import history)' '
+ git fast-import $TEST_DIRECTORY/t4211/history.export
+ git reset --hard
+'
+
+canned_test () {
+ test_expect_success $1
+ git log $1 actual
+ test_cmp \\$TEST_DIRECTORY\/t4211/expect.$2 actual
+
+}
+
+canned_test -L 4,12:a.c simple simple-f
+canned_test -L 4,+9:a.c simple simple-f
+canned_test -L '/long f/,/^}/:a.c' simple simple-f
+
+canned_test -L '/main/,/^}/:a.c' simple simple-main
+
+canned_test -L 1,+4:a.c simple beginning-of-file
+
+canned_test -L 20:a.c simple end-of-file
+
+test_expect_success -L 20,1:a.c (bogus end) '
+ test_must_fail git log -L 20,1:a.c simple 2errors
+ grep has only.*lines errors
+'
+
+canned_test -L '/long f/',/^}/:a.c -L /main/,/^}/:a.c simple two-ranges
+canned_test -L 24,+1:a.c simple vanishes-early
+
+canned_test -L '/long f/,/^}/:b.c' move-support move-support-f
+
+test_done
Hi Thomas,
You should probably add some failing tests (for example when the input
parameters are not correct), and as a matter of fact, I have the
following segfault with v9:
Starting program: /home/antoine/code/git/git-log -M -L :git.c
[Thread debugging using libthread_db enabled]
Using host libthread_db library /lib/x86_64-linux-gnu/libthread_db.so.1.
Program received signal SIGSEGV, Segmentation fault.
0x in ?? ()
(gdb) bt
#0 0x in ?? ()
#1 0x0050164d in parse_range_funcname (arg=0x84f2d0 :git.c,
nth_line_cb=0, cb_data=0x0, lines=0, begin=0x0, end=0x0, path=0x0)
at line-range.c:160
#2 0x00501d28 in skip_range_arg (arg=0x84f2d0 :git.c)
at line-range.c:229
#3 0x004fda78 in parse_lines (commit=0x86ea60, prefix=0x0,
args=0x7fad48) at line-log.c:544
#4 0x004fd75a in line_log_init (rev=0x7fffd9a8, prefix=0x0,
args=0x7fad48) at line-log.c:701
#5 0x00457189 in cmd_log_init_finish (argc=1, argv=0x7fffe238,
prefix=0x0, rev=0x7fffd9a8, opt=0x7fffd988) at builtin/log.c:192
#6 0x00455d2c in cmd_log_init (argc=4, argv=0x7fffe238,
prefix=0x0, rev=0x7fffd9a8, opt=0x7fffd988) at builtin/log.c:201
#7 0x00457250 in cmd_log (argc=4, argv=0x7fffe238, prefix=0x0)
at builtin/log.c:612
#8 0x004056cf in run_builtin (p=0x7f7af8, argc=4,
argv=0x7fffe238) at git.c:281
#9 0x004047aa in handle_internal_command (argc=4,
argv=0x7fffe238) at git.c:443
#10 0x00404506 in main (argc=4, argv=0x7fffe238) at git.c:532
--
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 v9a 3/5] Implement line-history search (git log -L)
You should probably add some failing tests I guess it should be failure tests, not failing tests. -- 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 v2] combine-diff: coalesce lost lines optimally
This replaces the greedy implementation to coalesce lost lines by using
dynamic programming to find the Longest Common Subsequence.
The O(n²) time complexity is obviously bigger than previous
implementation but it can produce shorter diff results (and most likely
easier to read).
List of lost lines is now doubly-linked because we reverse-read it when
reading the direction matrix.
Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
Hey,
This version includes some tests and is based on
ap/combine-diff-ignore-whitespace.
As you can see the last test is broken because the solution is not
optimal for more than two parents. It would probably require to extend
the dynamic programming to a k-dimension matrix (for k parents) but the
result would end-up being O(n^k) (when removing n consecutives lines
from p parents). I'm not sure there is any better solution known yet to
the k-LCS problem.
Implementing the dynamic solution with the k-dimension matrix would
probably require to re-hash the strings (I guess it's already done by
xdiff), as the number of string comparisons would increase.
Anyway I'm not exactly sure where to go from here :)
Cheers,
Antoine
combine-diff.c | 255 ++
t/t4038-diff-combined.sh | 129 +++
2 files changed, 320 insertions(+), 64 deletions(-)
diff --git a/combine-diff.c b/combine-diff.c
index 6288485..77d7872 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -74,16 +74,24 @@ static struct combine_diff_path *intersect_paths(struct
combine_diff_path *curr,
/* Lines lost from parent */
struct lline {
- struct lline *next;
+ struct lline *next, *prev;
int len;
unsigned long parent_map;
char line[FLEX_ARRAY];
};
+/* Lines lost from current parent (before coalescing) */
+struct plost {
+ struct lline *lost_head, *lost_tail;
+ int len;
+};
+
/* Lines surviving in the merge result */
struct sline {
- struct lline *lost_head, **lost_tail;
- struct lline *next_lost;
+ /* Accumulated and coalesced lost lines */
+ struct lline *lost;
+ int lenlost;
+ struct plost plost;
char *bol;
int len;
/* bit 0 up to (N-1) are on if the parent has this line (i.e.
@@ -95,34 +103,6 @@ struct sline {
unsigned long *p_lno;
};
-static char *grab_blob(const unsigned char *sha1, unsigned int mode,
- unsigned long *size, struct userdiff_driver *textconv,
- const char *path)
-{
- char *blob;
- enum object_type type;
-
- if (S_ISGITLINK(mode)) {
- blob = xmalloc(100);
- *size = snprintf(blob, 100,
-Subproject commit %s\n, sha1_to_hex(sha1));
- } else if (is_null_sha1(sha1)) {
- /* deleted blob */
- *size = 0;
- return xcalloc(1, 1);
- } else if (textconv) {
- struct diff_filespec *df = alloc_filespec(path);
- fill_filespec(df, sha1, 1, mode);
- *size = fill_textconv(textconv, df, blob);
- free_filespec(df);
- } else {
- blob = read_sha1_file(sha1, type, size);
- if (type != OBJ_BLOB)
- die(object '%s' is not a blob!, sha1_to_hex(sha1));
- }
- return blob;
-}
-
static int match_string_spaces(const char *line1, int len1,
const char *line2, int len2,
long flags)
@@ -163,36 +143,180 @@ static int match_string_spaces(const char *line1, int
len1,
return 0;
}
-static void append_lost(struct sline *sline, int n, const char *line, int len,
long flags)
+enum coalesce_direction { MATCH, BASE, NEW };
+
+/* Coalesce new lines into base by finding LCS */
+static struct lline *coalesce_lines(struct lline *base, int *lenbase,
+ struct lline *new, int lennew,
+ unsigned long parent, long flags)
{
- struct lline *lline;
- unsigned long this_mask = (1ULn);
- if (line[len-1] == '\n')
- len--;
+ int **lcs;
+ enum coalesce_direction **directions;
+ struct lline *baseend, *newend = NULL;
+ int i, j, origbaselen = *lenbase;
- /* Check to see if we can squash things */
- if (sline-lost_head) {
- lline = sline-next_lost;
- while (lline) {
- if (match_string_spaces(lline-line, lline-len,
- line, len, flags)) {
- lline-parent_map |= this_mask;
- sline-next_lost = lline-next;
- return;
+ if (new == NULL)
+ return base;
+
+ if (base == NULL) {
+ *lenbase = lennew;
+ return new
[PATCH] combine-diff: coalesce lost lines optimally
This replaces the greedy implementation to coalesce lost lines by using
dynamic programming to find the Longest Common Subsequence.
The O(n²) time complexity is obviously bigger than previous
implementation but it can produce shorter diff results (and most likely
easier to read).
List of lost lines is now doubly-linked because we reverse-read it when
reading the direction matrix.
Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
Hi,
This is a very first draft for improving the way we coalesce lost
lines. It has only been tested with the two scenarios below.
What is left to do:
- Test it more extensively
- Had some tests scenarios
I'm also having a hard time trying it with more than two parents. How I
am supposed to have more than two parents while octopus merge refuses if
there are conflicts ?
Tested scenarios:
git init
test
git add test
git commit -m initial
git checkout -b side1
seq 10 test
git commit -m all -a
git checkout master
seq 1 2 10 test
git commit -m three -a
git merge side1
test
git commit -m merge -a
git show
AND
git init
test
git add test
git commit -m initial
echo 3\n1\n2\n4 test
git commit -m shuffled -a
git checkout -b side HEAD^
echo 1\n2\n3\n4 test
git commit -m sorted -a
git merge master
test
git commit -m merge -a
git show
combine-diff.c | 192 ++--
1 file changed, 160 insertions(+), 32 deletions(-)
diff --git a/combine-diff.c b/combine-diff.c
index 35d41cd..252dd72 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -73,16 +73,24 @@ static struct combine_diff_path *intersect_paths(struct
combine_diff_path *curr,
/* Lines lost from parent */
struct lline {
- struct lline *next;
+ struct lline *next, *prev;
int len;
unsigned long parent_map;
char line[FLEX_ARRAY];
};
+/* Lines lost from current parent (before coalescing) */
+struct plost {
+ struct lline *lost_head, *lost_tail;
+ int len;
+};
+
/* Lines surviving in the merge result */
struct sline {
- struct lline *lost_head, **lost_tail;
- struct lline *next_lost;
+ /* Accumulated and coalesced lost lines */
+ struct lline *lost;
+ int lenlost;
+ struct plost plost;
char *bol;
int len;
/* bit 0 up to (N-1) are on if the parent has this line (i.e.
@@ -94,6 +102,132 @@ struct sline {
unsigned long *p_lno;
};
+enum coalesce_direction { MATCH, BASE, NEW };
+
+/* Coalesce new lines into base by finding LCS */
+static struct lline *coalesce_lines(struct lline *base, int *lenbase,
+ struct lline *new, int lennew,
+ unsigned long parent)
+{
+ int **lcs;
+ enum coalesce_direction **directions;
+ struct lline *baseend, *newend;
+ int i, j, origbaselen = *lenbase;
+
+ if (new == NULL)
+ return base;
+
+ if (base == NULL) {
+ *lenbase = lennew;
+ return new;
+ }
+
+ /*
+* Coalesce new lines into base by finding the LCS
+* - Create the table to run dynamic programing
+* - Compute the LCS
+* - Then reverse read the direction structure:
+* - If we have MATCH, assign parent to base flag, and consume
+* both baseend and newend
+* - Else if we have BASE, consume baseend
+* - Else if we have NEW, insert newend lline into base and
+* consume newend
+*/
+ lcs = xcalloc(origbaselen + 1, sizeof(int*));
+ directions = xcalloc(origbaselen + 1, sizeof(enum coalesce_direction*));
+ for (i = 0; i origbaselen + 1; i++) {
+ lcs[i] = xcalloc(lennew + 1, sizeof(int));
+ directions[i] = xcalloc(lennew + 1, sizeof(enum
coalesce_direction));
+ directions[i][0] = BASE;
+ }
+ for (j = 1; j lennew + 1; j++)
+ directions[0][j] = NEW;
+
+ for (i = 1, baseend = base; i origbaselen + 1; i++) {
+ for (j = 1, newend = new; j lennew + 1; j++) {
+ if (baseend-len == newend-len
+ !memcmp(baseend-line, newend-line, baseend-len))
{
+ lcs[i][j] = lcs[i - 1][j - 1] + 1;
+ directions[i][j] = MATCH;
+ } else if (lcs[i][j - 1] = lcs[i - 1][j]) {
+ lcs[i][j] = lcs[i][j - 1];
+ directions[i][j] = NEW;
+ } else {
+ lcs[i][j] = lcs[i - 1][j];
+ directions[i][j] = BASE;
+ }
+ if (newend-next)
+ newend = newend-next;
+ }
+ if (baseend-next)
+ baseend = baseend-next;
+ }
+
+ for (i = 0; i origbaselen + 1; i++)
+ free(lcs[i
Re: [PATCH] combine-diff: coalesce lost lines optimally
I'm also having a hard time trying it with more than two parents. How I am supposed to have more than two parents while octopus merge refuses if there are conflicts ? OK, creating the merge commit myself solves the issue: git init test git add test git commit -m initial seq 100 test git commit -m all -a git checkout -b side1 HEAD^1 seq 1 2 100 test git commit -m side1 -a git checkout -b side2 HEAD^1 seq 1 4 100 test git commit -m side2 -a git checkout -b side3 HEAD^1 seq 1 8 100 test git commit -m side3 -a git checkout -b side4 HEAD^1 seq 1 16 100 test git commit -m side4 -a git checkout master test git add test TREE=$(git write-tree) COMMIT=$(git commit-tree $TREE -p master -p side1 -p side2 -p side3 -p side4 -m merge) git show $COMMIT This will work with the basic greedy implementation if all parents are in this order. But the optimal result will be lost if we change the order of -p parameters in git-commit-tree. The patch seems to be correct, always finding the best result (we always have 100 lines diff) whatever the order of parents. -- 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] combine-diff: coalesce lost lines optimally
Hopefully, my patch takes about the same time as git 1.7.9.5 and produces the same output on that commit ;) Unfortunately on a commit that would remove A LOT of lines (1) from 7 parents, the times goes from 0.01s to 1.5s... I'm pretty sure that scenario is quite uncommon though. On Sun, Mar 17, 2013 at 9:10 PM, Junio C Hamano gits...@pobox.com wrote: Antoine Pelisse apeli...@gmail.com writes: This replaces the greedy implementation to coalesce lost lines by using dynamic programming to find the Longest Common Subsequence. The O(n²) time complexity is obviously bigger than previous implementation but it can produce shorter diff results (and most likely easier to read). List of lost lines is now doubly-linked because we reverse-read it when reading the direction matrix. Signed-off-by: Antoine Pelisse apeli...@gmail.com --- Hi, This is a very first draft for improving the way we coalesce lost lines. It has only been tested with the two scenarios below. What is left to do: - Test it more extensively - Had some tests scenarios I'm also having a hard time trying it with more than two parents. How I am supposed to have more than two parents while octopus merge refuses if there are conflicts ? 9fdb62af92c7 ([ACPI] merge 3549 4320 4485 4588 4980 5483 5651 acpica asus fops pnpacpi branches into release, 2006-01-24) is one of the amusing examples ;-) Cf. http://thread.gmane.org/gmane.comp.version-control.git/15486 -- 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] Allow combined diff to ignore white-spaces
On Thu, Mar 14, 2013 at 4:31 PM, Junio C Hamano gits...@pobox.com wrote: Johannes Sixt j.s...@viscovery.net writes: This form of 'echo' is not sufficiently portable. How about: tr -d Q -\EOF test always coalesce eol space coalesce Q ... EOF Much better. +test_expect_success 'check combined output (no ignore space)' ' +git show | test_i18ngrep ^-\s*eol spaces ... +git show | test_i18ngrep ^--\s*always coalesce This loses the exit code of git show. We usually write this as git show actual grep ^- *eol spaces grep ^- *eol space coalesce ... (Same for later tests.) There is nothing i18n-ish in the test patterns. Use regular grep. BTW, there is compare_diff_patch() in diff-lib.sh. You can use it to compare diff output to expected output. Then you do not need a grep invocation for each line of the test file. All good suggestions. Thanks. OK Very good, I will resubmit tonight. I was indeed not really sure about the best way to test here. Thanks for the tips and confirmation ! Cheers, -- 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] Allow combined diff to ignore white-spaces
Currently, it's not possible to use the space-ignoring options (-b, -w,
--ignore-space-at-eol) with combined diff. It makes it pretty impossible
to read a merge between a branch that changed all tabs to spaces, and a
branch with functional changes.
Pass diff flags to diff engine, so that combined diff behaves as normal
diff does with spaces.
Also coalesce lines that are removed from both (or more) parents.
It also means that a conflict-less merge done using a ignore-* strategy
option will not show any conflict if shown in combined-diff using the
same option.
Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
Changes:
- Fixed comments
- Fixed tests (following Johannes suggestions)
combine-diff.c | 57 +---
t/t4038-diff-combined.sh | 111 ++
2 files changed, 161 insertions(+), 7 deletions(-)
diff --git a/combine-diff.c b/combine-diff.c
index 35d41cd..6288485 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -5,6 +5,7 @@
#include diffcore.h
#include quote.h
#include xdiff-interface.h
+#include xdiff/xmacros.h
#include log-tree.h
#include refs.h
#include userdiff.h
@@ -122,7 +123,47 @@ static char *grab_blob(const unsigned char *sha1, unsigned
int mode,
return blob;
}
-static void append_lost(struct sline *sline, int n, const char *line, int len)
+static int match_string_spaces(const char *line1, int len1,
+ const char *line2, int len2,
+ long flags)
+{
+ if (flags XDF_WHITESPACE_FLAGS) {
+ for (; len1 0 XDL_ISSPACE(line1[len1 - 1]); len1--);
+ for (; len2 0 XDL_ISSPACE(line2[len2 - 1]); len2--);
+ }
+
+ if (!(flags (XDF_IGNORE_WHITESPACE | XDF_IGNORE_WHITESPACE_CHANGE)))
+ return (len1 == len2 !memcmp(line1, line2, len1));
+
+ while (len1 0 len2 0) {
+ len1--;
+ len2--;
+ if (XDL_ISSPACE(line1[len1]) || XDL_ISSPACE(line2[len2])) {
+ if ((flags XDF_IGNORE_WHITESPACE_CHANGE)
+ (!XDL_ISSPACE(line1[len1]) ||
!XDL_ISSPACE(line2[len2])))
+ return 0;
+
+ for (; len1 0 XDL_ISSPACE(line1[len1]); len1--);
+ for (; len2 0 XDL_ISSPACE(line2[len2]); len2--);
+ }
+ if (line1[len1] != line2[len2])
+ return 0;
+ }
+
+ if (flags XDF_IGNORE_WHITESPACE) {
+ /* Consume remaining spaces */
+ for (; len1 0 XDL_ISSPACE(line1[len1 - 1]); len1--);
+ for (; len2 0 XDL_ISSPACE(line2[len2 - 1]); len2--);
+ }
+
+ /* We matched full line1 and line2 */
+ if (!len1 !len2)
+ return 1;
+
+ return 0;
+}
+
+static void append_lost(struct sline *sline, int n, const char *line, int len,
long flags)
{
struct lline *lline;
unsigned long this_mask = (1ULn);
@@ -133,8 +174,8 @@ static void append_lost(struct sline *sline, int n, const
char *line, int len)
if (sline-lost_head) {
lline = sline-next_lost;
while (lline) {
- if (lline-len == len
- !memcmp(lline-line, line, len)) {
+ if (match_string_spaces(lline-line, lline-len,
+ line, len, flags)) {
lline-parent_map |= this_mask;
sline-next_lost = lline-next;
return;
@@ -162,6 +203,7 @@ struct combine_diff_state {
int n;
struct sline *sline;
struct sline *lost_bucket;
+ long flags;
};
static void consume_line(void *state_, char *line, unsigned long len)
@@ -201,7 +243,7 @@ static void consume_line(void *state_, char *line, unsigned
long len)
return; /* not in any hunk yet */
switch (line[0]) {
case '-':
- append_lost(state-lost_bucket, state-n, line+1, len-1);
+ append_lost(state-lost_bucket, state-n, line+1, len-1,
state-flags);
break;
case '+':
state-sline[state-lno-1].flag |= state-nmask;
@@ -215,7 +257,7 @@ static void combine_diff(const unsigned char *parent,
unsigned int mode,
struct sline *sline, unsigned int cnt, int n,
int num_parent, int result_deleted,
struct userdiff_driver *textconv,
-const char *path)
+const char *path, long flags)
{
unsigned int p_lno, lno;
unsigned long nmask = (1UL n);
@@ -231,9 +273,10 @@ static void combine_diff(const unsigned char *parent,
unsigned int mode,
parent_file.ptr = grab_blob(parent, mode, sz, textconv, path);
parent_file.size = sz;
memset(xpp, 0
[PATCH] Allow combined diff to ignore white-spaces
Currently, it's not possible to use the space-ignoring options (-b, -w,
--ignore-space-at-eol) with combined diff. It makes it pretty impossible
to read a merge between a branch that changed all tabs to spaces, and a
branch with functional changes.
Pass diff flags to diff engine, so that combined diff behaves as normal
diff does with spaces.
Also coalesce lines that are removed from both (or more) parents.
It also means that a conflict-less merge done using a ignore-* strategy
option will not show any conflict if shown in combined-diff using the
same option.
Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
OK, I added some tests and coalesce similar lost lines (using the same flags
we used for diff.
combine-diff.c | 57 ++-
t/t4038-diff-combined.sh | 75 ++
2 files changed, 125 insertions(+), 7 deletions(-)
diff --git a/combine-diff.c b/combine-diff.c
index 35d41cd..0f33983 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -5,6 +5,7 @@
#include diffcore.h
#include quote.h
#include xdiff-interface.h
+#include xdiff/xmacros.h
#include log-tree.h
#include refs.h
#include userdiff.h
@@ -122,7 +123,47 @@ static char *grab_blob(const unsigned char *sha1, unsigned
int mode,
return blob;
}
-static void append_lost(struct sline *sline, int n, const char *line, int len)
+static int match_string_spaces(const char *line1, int len1,
+ const char *line2, int len2,
+ long flags)
+{
+ if (flags XDF_WHITESPACE_FLAGS) {
+ for (; len1 0 XDL_ISSPACE(line1[len1 - 1]); len1--);
+ for (; len2 0 XDL_ISSPACE(line2[len2 - 1]); len2--);
+ }
+
+ if (!(flags (XDF_IGNORE_WHITESPACE | XDF_IGNORE_WHITESPACE_CHANGE)))
+ return (len1 == len2 !memcmp(line1, line2, len1));
+
+ while (len1 0 len2 0) {
+ len1--;
+ len2--;
+ if (XDL_ISSPACE(line1[len1]) || XDL_ISSPACE(line2[len2])) {
+ if ((flags XDF_IGNORE_WHITESPACE_CHANGE)
+ (!XDL_ISSPACE(line1[len1]) ||
!XDL_ISSPACE(line2[len2])))
+ return 0;
+
+ for (; len1 0 XDL_ISSPACE(line1[len1]); len1--);
+ for (; len2 0 XDL_ISSPACE(line2[len2]); len2--);
+ }
+ if (line1[len1] != line2[len2])
+ return 0;
+ }
+
+ if (flags XDF_IGNORE_WHITESPACE) {
+ // Consume remaining spaces
+ for (; len1 0 XDL_ISSPACE(line1[len1 - 1]); len1--);
+ for (; len2 0 XDL_ISSPACE(line2[len2 - 1]); len2--);
+ }
+
+ // We matched full line1 and line2
+ if (!len1 !len2)
+ return 1;
+
+ return 0;
+}
+
+static void append_lost(struct sline *sline, int n, const char *line, int len,
long flags)
{
struct lline *lline;
unsigned long this_mask = (1ULn);
@@ -133,8 +174,8 @@ static void append_lost(struct sline *sline, int n, const
char *line, int len)
if (sline-lost_head) {
lline = sline-next_lost;
while (lline) {
- if (lline-len == len
- !memcmp(lline-line, line, len)) {
+ if (match_string_spaces(lline-line, lline-len,
+ line, len, flags)) {
lline-parent_map |= this_mask;
sline-next_lost = lline-next;
return;
@@ -162,6 +203,7 @@ struct combine_diff_state {
int n;
struct sline *sline;
struct sline *lost_bucket;
+ long flags;
};
static void consume_line(void *state_, char *line, unsigned long len)
@@ -201,7 +243,7 @@ static void consume_line(void *state_, char *line, unsigned
long len)
return; /* not in any hunk yet */
switch (line[0]) {
case '-':
- append_lost(state-lost_bucket, state-n, line+1, len-1);
+ append_lost(state-lost_bucket, state-n, line+1, len-1,
state-flags);
break;
case '+':
state-sline[state-lno-1].flag |= state-nmask;
@@ -215,7 +257,7 @@ static void combine_diff(const unsigned char *parent,
unsigned int mode,
struct sline *sline, unsigned int cnt, int n,
int num_parent, int result_deleted,
struct userdiff_driver *textconv,
-const char *path)
+const char *path, long flags)
{
unsigned int p_lno, lno;
unsigned long nmask = (1UL n);
@@ -231,9 +273,10 @@ static void combine_diff(const unsigned char *parent,
unsigned int mode,
parent_file.ptr = grab_blob(parent, mode, sz, textconv, path);
parent_file.size
Re: [PATCH v3 04/13] match_basename: use strncmp instead of strcmp
--- a/dir.c
+++ b/dir.c
@@ -636,12 +636,14 @@ int match_basename(const char *basename, int
basenamelen,
int flags)
{
if (prefix == patternlen) {
- if (!strcmp_icase(pattern, basename))
+ if (patternlen == basenamelen
+ !strncmp_icase(pattern, basename, patternlen))
return 1;
} else if (flags EXC_FLAG_ENDSWITH) {
if (patternlen - 1 = basenamelen
- !strcmp_icase(pattern + 1,
- basename + basenamelen - patternlen + 1))
+ !strncmp_icase(pattern + 1,
+ basename + basenamelen - patternlen + 1,
+ patternlen - 1))
return 1;
I can see that you kept strncmp(), was it worse with memcmp() ?
--
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 v2 3/6] match_basename: use strncmp instead of strcmp
On Sun, Mar 10, 2013 at 11:38 AM, Duy Nguyen pclo...@gmail.com wrote: glibc's C strncmp version does 4-byte comparison at a time when n =4, then fall back to 1-byte for the rest. Looking at this (http://fossies.org/dox/glibc-2.17/strncmp_8c_source.html), it's not exactly true. It would rather be while (n = 4), manually unroll the loop. -- 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 v2 3/6] match_basename: use strncmp instead of strcmp
On Sun, Mar 10, 2013 at 12:43 PM, Antoine Pelisse apeli...@gmail.com wrote: On Sun, Mar 10, 2013 at 11:38 AM, Duy Nguyen pclo...@gmail.com wrote: glibc's C strncmp version does 4-byte comparison at a time when n =4, then fall back to 1-byte for the rest. Looking at this (http://fossies.org/dox/glibc-2.17/strncmp_8c_source.html), it's not exactly true. It would rather be while (n = 4), manually unroll the loop. By the way, if we know the length of the string, we could use memcmp. This one is allowed to compare 4-bytes at a time (he doesn't care about end of string). This is true because the value of the length parameter is no longer at most. -- 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 v2 3/6] match_basename: use strncmp instead of strcmp
By the way, if we know the length of the string, we could use memcmp. This one is allowed to compare 4-bytes at a time (he doesn't care about end of string). This is true because the value of the length parameter is no longer at most. We still need to worry about access violation after NUL when two strings have different lengths. That could be avoided in this particular case, but I think it's too fragile. Why would we need to compare if the strings don't have the same length ? We already do that in combine-diff.c:append_lost(). -- 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 1/3] match_pathname: avoid calling strncmp if baselen is 0
diff --git a/dir.c b/dir.c index 57394e4..669cf80 100644 --- a/dir.c +++ b/dir.c @@ -663,7 +663,7 @@ int match_pathname(const char *pathname, int pathlen, */ if (pathlen baselen + 1 || (baselen pathname[baselen] != '/') || - strncmp_icase(pathname, base, baselen)) + (baselen strncmp_icase(pathname, base, baselen))) Shouldn't you factorize by baselen here ? For readability reasons, not performance of course. return 0; namelen = baselen ? pathlen - baselen - 1 : pathlen; -- 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] perf: update documentation of GIT_PERF_REPEAT_COUNT
Currently the documentation of GIT_PERF_REPEAT_COUNT says the default is five while perf-lib.sh uses a value of three as a default. Update the documentation so that it is consistent with the code. Signed-off-by: Antoine Pelisse apeli...@gmail.com --- t/perf/README |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/perf/README b/t/perf/README index b2dbad4..c552f56 100644 --- a/t/perf/README +++ b/t/perf/README @@ -56,7 +56,7 @@ You can set the following variables (also in your config.mak): GIT_PERF_REPEAT_COUNT Number of times a test should be repeated for best-of-N - measurements. Defaults to 5. + measurements. Defaults to 3. GIT_PERF_MAKE_OPTS Options to use when automatically building a git tree for -- 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: feature suggestion: optimize common parts for checkout --conflict=diff3
git checkout --conflict=diff3 file
That's somehow unrelated, but shouldn't we have a conflict option to
git-merge as we have for git-checkout ?
With something like this (pasted into gmail):
diff --git a/builtin/merge.c b/builtin/merge.c
index 7c8922c..edad742 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -65,6 +65,7 @@ static int abort_current_merge;
static int show_progress = -1;
static int default_to_upstream;
static const char *sign_commit;
+static char *conflict_style;
static struct strategy all_strategy[] = {
{ recursive, DEFAULT_TWOHEAD | NO_TRIVIAL },
@@ -213,6 +214,7 @@ static struct option builtin_merge_options[] = {
{ OPTION_STRING, 'S', gpg-sign, sign_commit, N_(key id),
N_(GPG sign commit), PARSE_OPT_OPTARG, NULL, (intptr_t) },
OPT_BOOLEAN(0, overwrite-ignore, overwrite_ignore, N_(update
ignored files (default))),
+ OPT_STRING(0, conflict, conflict_style, N_(style), N_(conflict
style (merge or diff3))),
OPT_END()
};
@@ -1102,6 +1104,9 @@ int cmd_merge(int argc, const char **argv, const
char *prefix)
if (verbosity 0 show_progress == -1)
show_progress = 0;
+ if (conflict_style)
+ git_xmerge_config(merge.conflictstyle, conflict_style, NULL);
+
if (abort_current_merge) {
int nargc = 2;
const char *nargv[] = {reset, --merge, NULL};
--
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: feature suggestion: optimize common parts for checkout --conflict=diff3
however the difference isn't that easy to spot any more. I expected diff --cc file index a07e697,5080129..000 --- a/file +++ b/file @@@ -12,7 -12,7 +12,12 @@@ 12 13 14 ++ ours +15 ++||| base ++=== + 16 ++ theirs 17 18 19 This is not correct, it would mean that 12, 13, 14, 17, 18, 19 are in base, while they are not. Cheers, Antoine -- 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 v2] tests: make sure rename pretty print works
Add basic use cases and corner cases tests for
git diff -M --summary/stat.
Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
list of fixes:
- Test using diff instead of show
(that is more consistent with commit message).
- add extra spaces around paths
- Use better commit messages
- Moved to existing t4001
t/t4001-diff-rename.sh | 54
1 file changed, 54 insertions(+)
diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh
index 844277c..2f327b7 100755
--- a/t/t4001-diff-rename.sh
+++ b/t/t4001-diff-rename.sh
@@ -102,4 +102,58 @@ test_expect_success 'setup for many rename source
candidates' '
grep warning actual.err
'
+test_expect_success 'rename pretty print with nothing in common' '
+ mkdir -p a/b/
+ : a/b/c
+ git add a/b/c
+ git commit -m create a/b/c
+ mkdir -p c/b/
+ git mv a/b/c c/b/a
+ git commit -m a/b/c - c/b/a
+ git diff -M --summary HEAD^ HEAD output
+ test_i18ngrep a/b/c = c/b/a output
+ git diff -M --stat HEAD^ HEAD output
+ test_i18ngrep a/b/c = c/b/a output
+'
+
+test_expect_success 'rename pretty print with common prefix' '
+ mkdir -p c/d
+ git mv c/b/a c/d/e
+ git commit -m c/b/a - c/d/e
+ git diff -M --summary HEAD^ HEAD output
+ test_i18ngrep c/{b/a = d/e} output
+ git diff -M --stat HEAD^ HEAD output
+ test_i18ngrep c/{b/a = d/e} output
+'
+
+test_expect_success 'rename pretty print with common suffix' '
+ mkdir d
+ git mv c/d/e d/e
+ git commit -m c/d/e - d/e
+ git diff -M --summary HEAD^ HEAD output
+ test_i18ngrep {c/d = d}/e output
+ git diff -M --stat HEAD^ HEAD output
+ test_i18ngrep {c/d = d}/e output
+'
+
+test_expect_success 'rename pretty print with common prefix and suffix' '
+ mkdir d/f
+ git mv d/e d/f/e
+ git commit -m d/e - d/f/e
+ git diff -M --summary HEAD^ HEAD output
+ test_i18ngrep d/{ = f}/e output
+ git diff -M --stat HEAD^ HEAD output
+ test_i18ngrep d/{ = f}/e output
+'
+
+test_expect_success 'rename pretty print common prefix and suffix overlap' '
+ mkdir d/f/f
+ git mv d/f/e d/f/f/e
+ git commit -m d/f/e d/f/f/e
+ git diff -M --summary HEAD^ HEAD output
+ test_i18ngrep d/f/{ = f}/e output
+ git diff -M --stat HEAD^ HEAD output
+ test_i18ngrep d/f/{ = f}/e output
+'
+
test_done
--
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] Allow combined diff to ignore white-spaces
That should be reviewed carefully as I'm not exactly sure that does make sense with the way combined-diff works. Still it seems natural to me to be able to remove the space in combined diff as we do with normal diff. Especially as I unfortunately have to deal with many space + feature merges that are very hard to analyze/handle if space differences can't be hidden. Assuming That at the beginning of the off-line comment refers to this patch, I actually I do not think this patch needs to be reviewed carefully at all. I actually meant: is that patch enough or is there anything I missed. Considering your answer, it looks like I did ;) An example. After collecting pairwise diffs between the shared postimage and individual parents, we align the hunks and coalesce identical preimage lines to form shared - (removed) lines. I think that step is done with byte-for-byte comparison. If the postimage removes a line from one parent and a corresponding line from another parent, and if these corresponding lines in the parents differ only by whitespaces in a way the user told us to ignore with -b/-w/etc., you would need to tweak the logic to coalesce corresponding lost lines in the append_lost() function. Otherwise, you will see two - and - lines that remove these corresponding lines from the parents that are identical when you ignore whitespaces. That is the part I missed, and it's nicely explained. You should add some tests; it would help you think about these issues through. I will definitely add some tests. For example, in this history: git init seq 11 ten git add ten git commit -m initial seq 10 | sed -e 's/6/6 six/' -e 's/2/2 two/' ten echo 11 ten git commit -m ten -a git checkout -b side HEAD^ seq 10 | sed -e 's/^/ /' | sed -e 's/2/2 dos/' ten echo 11 ten git commit -m indent -a git merge master seq 10 | sed -e 's/5/5 go/' -e 's/2/2 dois/' ten git commit -m merged -a one side indents the original and tweaks some lines, while the other side tweaks some other lines without reindenting. Most notably, the fifth line on one side is 5 while on the other side is 5, and this line is rewritten to 5 go in the final. The lost lines are not coalesced to a single -- 5 (nor -- 5) but shows that two preimages were different only by whitespaces. Compare that with what happens to the final line 11 that gets lost in the result. It feels incorrect to me to coalsesce - 5 and - 5 as it might look incorrect to the user. But still the idea is appealing. I have an implementation for that that requires more testing. Using the exact example you gave, and running the latest next, I have this output, where 11 is not coalesced. Is that a bug ? diff --cc ten index d213a99,ed40ab2..37c2af2 --- a/ten +++ b/ten @@@ -1,11 -1,11 +1,10 @@@ - 1 - 2 dos - 3 - 4 - 5 - 6 - 7 - 8 - 9 - 10 - 11 + 1 -2 two ++2 dois + 3 + 4 -5 -6 six ++5 go ++6 + 7 + 8 + 9 + 10 -11 -- 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] Allow combined diff to ignore white-spaces
On Mon, Mar 4, 2013 at 7:36 PM, Junio C Hamano gits...@pobox.com wrote: Antoine Pelisse apeli...@gmail.com writes: It feels incorrect to me to coalsesce - 5 and - 5 as it might look incorrect to the user. But still the idea is appealing. The users already need to see that when reading a regular patch with one or more context lines and -b/-w/etc., anyway. The context lines are made into context only because whitespace differences were ignored, and in the regular unified patch format we can show only one version, either from preimage or from postimage, and have to pick one. Coalescing - 5 and - 5 into --5 or -- 5 by picking one or the other is the same thing, no? That's all I needed to be convinced. I obviously don't care which one we pick. Using the exact example you gave, and running the latest next, I have this output, where 11 is not coalesced. Is that a bug ? It could be tickling a corner case because the removal is at the end of the file. Perhaps adding 12 that is all common across three versions and see what happens? Doesn't make a difference. Still have - 11 and -11. I will try to have a look at it. -- 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] tests: make sure rename pretty print works
Add basic use cases and corner cases tests for
git diff -M --summary/stat.
Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
t/t4056-rename-pretty.sh | 54 ++
1 file changed, 54 insertions(+)
create mode 100755 t/t4056-rename-pretty.sh
diff --git a/t/t4056-rename-pretty.sh b/t/t4056-rename-pretty.sh
new file mode 100755
index 000..806046f
--- /dev/null
+++ b/t/t4056-rename-pretty.sh
@@ -0,0 +1,54 @@
+#!/bin/sh
+
+test_description='Rename pretty print
+
+'
+
+. ./test-lib.sh
+
+test_expect_success nothing_common '
+ mkdir -p a/b/
+ : a/b/c
+ git add a/b/c
+ git commit -m.
+ mkdir -p c/b/
+ git mv a/b/c c/b/a
+ git commit -m.
+ git show -M --summary output
+ test_i18ngrep a/b/c = c/b/a output
+'
+
+test_expect_success common_prefix '
+ mkdir -p c/d
+ git mv c/b/a c/d/e
+ git commit -m.
+ git show -M --summary output
+ test_i18ngrep c/{b/a = d/e} output
+'
+
+test_expect_success common_suffix '
+ mkdir d
+ git mv c/d/e d/e
+ git commit -m.
+ git show -M --summary output
+ test_i18ngrep {c/d = d}/e output
+'
+
+test_expect_success common_suffix_prefix '
+ mkdir d/f
+ git mv d/e d/f/e
+ git commit -m.
+ git show -M --summary output
+ test_i18ngrep d/{ = f}/e output
+'
+
+test_expect_success common_overlap '
+ mkdir d/f/f
+ git mv d/f/e d/f/f/e
+ git commit -m.
+ git show -M --summary output
+ test_i18ngrep d/f/{ = f}/e output
+'
+
+
+test_done
--
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] Allow combined diff to ignore white-spaces
Currently, it's not possible to use the space-ignoring options (-b, -w,
--ignore-space-at-eol) with combined diff. It makes it pretty impossible
to read a merge between a branch that changed all tabs to spaces, and a
branch with functional changes.
Pass diff flags to diff engine, so that combined diff behaves as normal
diff does with spaces.
It also means that a conflict-less merge done using a ignore-* strategy
option will not show any conflict if shown in combined-diff using the
same option.
Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
That should be reviewed carefully as I'm not exactly sure that does make
sense with the way combined-diff works.
Still it seems natural to me to be able to remove the space in combined
diff as we do with normal diff. Especially as I unfortunately have to
deal with many space + feature merges that are very hard to analyze/handle
if space differences can't be hidden.
Cheers,
Antoine
combine-diff.c |6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/combine-diff.c b/combine-diff.c
index 35d41cd..7ca0a72 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -215,7 +215,7 @@ static void combine_diff(const unsigned char *parent,
unsigned int mode,
struct sline *sline, unsigned int cnt, int n,
int num_parent, int result_deleted,
struct userdiff_driver *textconv,
-const char *path)
+const char *path, long flags)
{
unsigned int p_lno, lno;
unsigned long nmask = (1UL n);
@@ -231,7 +231,7 @@ static void combine_diff(const unsigned char *parent,
unsigned int mode,
parent_file.ptr = grab_blob(parent, mode, sz, textconv, path);
parent_file.size = sz;
memset(xpp, 0, sizeof(xpp));
- xpp.flags = 0;
+ xpp.flags = flags;
memset(xecfg, 0, sizeof(xecfg));
memset(state, 0, sizeof(state));
state.nmask = nmask;
@@ -962,7 +962,7 @@ static void show_patch_diff(struct combine_diff_path *elem,
int num_parent,
elem-parent[i].mode,
result_file, sline,
cnt, i, num_parent, result_deleted,
-textconv, elem-path);
+textconv, elem-path, opt-xdl_opts);
}
show_hunks = make_hunks(sline, cnt, num_parent, dense);
--
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] diff: Fix rename pretty-print when suffix and prefix overlap
When considering a rename for two files that have a suffix and a prefix
that can overlap, a confusing line is shown. As an example, renaming
a/b/b/c to a/b/c shows a/b/{ = }/b/c, instead of a/b/{b = }/c
Currently, what we do is calculate the common prefix (a/b/), and the
common suffix (/b/c), but the same /b/ is actually counted both in
prefix and suffix. Then when calculating the size of the non-common part,
we end-up with a negative value which is reset to 0, thus the { = }.
Do not allow the common suffix to overlap the common prefix and stop
when reaching a / that would be in both.
Also add some test file to place corner-cases we could met (and this one)
with rename pretty print.
Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
diff.c | 11 +-
t/t4056-rename-pretty.sh | 54 ++
2 files changed, 64 insertions(+), 1 deletion(-)
create mode 100755 t/t4056-rename-pretty.sh
diff --git a/diff.c b/diff.c
index 9038f19..e1d82c9 100644
--- a/diff.c
+++ b/diff.c
@@ -1177,7 +1177,16 @@ static char *pprint_rename(const char *a, const char *b)
old = a + len_a;
new = b + len_b;
sfx_length = 0;
- while (a = old b = new *old == *new) {
+ /*
+* Note:
+* if pfx_length is 0, old/new will never reach a - 1 because it
+* would mean the whole string is common suffix. But then, the
+* whole string would also be a common prefix, and we would not
+* have pfx_length equals 0.
+*/
+ while (a + pfx_length - 1 = old
+ b + pfx_length - 1 = new
+ *old == *new) {
if (*old == '/')
sfx_length = len_a - (old - a);
old--;
diff --git a/t/t4056-rename-pretty.sh b/t/t4056-rename-pretty.sh
new file mode 100755
index 000..806046f
--- /dev/null
+++ b/t/t4056-rename-pretty.sh
@@ -0,0 +1,54 @@
+#!/bin/sh
+
+test_description='Rename pretty print
+
+'
+
+. ./test-lib.sh
+
+test_expect_success nothing_common '
+ mkdir -p a/b/
+ : a/b/c
+ git add a/b/c
+ git commit -m.
+ mkdir -p c/b/
+ git mv a/b/c c/b/a
+ git commit -m.
+ git show -M --summary output
+ test_i18ngrep a/b/c = c/b/a output
+'
+
+test_expect_success common_prefix '
+ mkdir -p c/d
+ git mv c/b/a c/d/e
+ git commit -m.
+ git show -M --summary output
+ test_i18ngrep c/{b/a = d/e} output
+'
+
+test_expect_success common_suffix '
+ mkdir d
+ git mv c/d/e d/e
+ git commit -m.
+ git show -M --summary output
+ test_i18ngrep {c/d = d}/e output
+'
+
+test_expect_success common_suffix_prefix '
+ mkdir d/f
+ git mv d/e d/f/e
+ git commit -m.
+ git show -M --summary output
+ test_i18ngrep d/{ = f}/e output
+'
+
+test_expect_success common_overlap '
+ mkdir d/f/f
+ git mv d/f/e d/f/f/e
+ git commit -m.
+ git show -M --summary output
+ test_i18ngrep d/f/{ = f}/e output
+'
+
+
+test_done
--
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] diff: Fix rename pretty-print when suffix and prefix overlap
On Thu, Feb 28, 2013 at 11:14 PM, Thomas Rast tr...@student.ethz.ch wrote:
Antoine Pelisse apeli...@gmail.com writes:
diff --git a/diff.c b/diff.c
index 9038f19..e1d82c9 100644
--- a/diff.c
+++ b/diff.c
@@ -1177,7 +1177,16 @@ static char *pprint_rename(const char *a, const char
*b)
- while (a = old b = new *old == *new) {
+ /*
+ * Note:
+ * if pfx_length is 0, old/new will never reach a - 1 because it
+ * would mean the whole string is common suffix. But then, the
+ * whole string would also be a common prefix, and we would not
+ * have pfx_length equals 0.
+ */
+ while (a + pfx_length - 1 = old
+b + pfx_length - 1 = new
+*old == *new) {
Umm, you still have the broken version here, and the previous patch is
already in next. I think you should decide for one thing ;-)
Thanks ! I had not paid enough attention to that.
Either: consider this a reroll; Junio would have to revert the version
already in next (which isn't _so_ bad, because next will eventually be
rebuilt) and apply this new version. But if you do that, you should
squash my change that deals with the underrun issue (I'd be fine with
that).
I would not have done that without your consent (that's why I kept the
buggy version)
Or: consider it an incremental improvement on the series, in which case
you should send only the tests with a new commit message.
That seems like the best solution to me. I will resend later with just
the tests and a new commit message.
Cheers,
Antoine
--
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] diff: prevent pprint_rename from underrunning input
The logic described in d020e27 (diff: Fix rename pretty-print when suffix and prefix overlap, 2013-02-23) is wrong: The proof in the comment is valid only if both strings are the same length. *One* of old/new can reach a-1 (b-1, resp.) if 'a' is a suffix of 'b' (or vice versa). Indeed, sorry about that. I guess I was too focused on my specific issue and didn't broaden my thoughts. Thanks. I was also having hard time convincing myself why that -1 does not under-run yesterday. Will queue. 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: [RFC] git rm -u
I must say that I'm not very interested in the feature. In my opinion, there are already many different ways to stage changes. Assuming that the feature would be needed, I would keep it under the scope of git-add, as it's the reference for staging. I would suggest something like: git add -r Stage removal of deleted files. On Mon, Feb 25, 2013 at 7:54 AM, Junio C Hamano gits...@pobox.com wrote: Eric James Michael Ritz lobbyjo...@gmail.com writes: On 01/19/2013 04:49 PM, Antoine Pelisse wrote: I think `git add -u` would be closer. It would stage removal of files, but would not stage untracked files. It would stage other type of changes though. On Sat, Jan 19, 2013 at 10:47 PM, Tomas Carnecky Does `git add -A` do what you want? Thank you Tomas and Antoine. Both of these commands do what I want: stage deleted files on the index. But does the idea of a `git rm -u` still sound useful since these commands also stage changes besides deleted files? Even though I am not sure how often I would use it myself, reflect only the removals in the working tree to the index, but exclude any other kind of changes might turn out to be a useful addition to the toolchest in certain cases. I however am not yet convinced that git rm -u is a good way to express the feature at the UI. git add -u is update the index with modification and removal but ignore new files because we won't know if they are garbage or assets. What the same -u option means in the context of git rm is not very clear, at least to me. -- 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: [RFC] git rm -u
On Mon, Feb 25, 2013 at 8:07 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Antoine Pelisse apeli...@gmail.com writes: I must say that I'm not very interested in the feature. In my opinion, there are already many different ways to stage changes. Assuming that the feature would be needed, I would keep it under the scope of git-add, as it's the reference for staging. I would suggest something like: git add -r Stage removal of deleted files. Would add -r stand for add --remove? That would be weird ... Yes (for --remove), It would not be weird if you consider the opposite of add to be reset (and not rm). git rm really seems to be a better place for removing files from the index. Then, I don't exactly understand the meaning of git-rm but being a _shortcut_ for remove and stage. Here the files are already removed, we only need to stage and the best command to stage changes is add. -- 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] diff: Fix rename pretty-print when suffix and prefix overlap
On Sun, Feb 24, 2013 at 10:15 AM, Junio C Hamano gits...@pobox.com wrote:
Antoine Pelisse apeli...@gmail.com writes:
When considering a rename for two files that have a suffix and a prefix
that can overlap, a confusing line is shown. As an example, renaming
a/b/b/c to a/b/c shows a/b/{ = }/b/c.
This would be vastly more readable if it had It should show XXX
instead somewhere in the description, perhaps at the end of this
sentence. It can also be after thus the { = } below, but I think
giving the expected output earlier would be more appropriate.
Good catch, this would probably be better:
When considering a rename for two files that have a suffix and a prefix
that can overlap, a confusing line is shown. As an example, renaming
a/b/b/c to a/b/c shows a/b/{ = }/b/c, instead of a/b/{ = b}/c
Currently, what we do is calculate the common prefix (a/b/), and the
common suffix (/b/c), but the same /b/ is actually counted both in
prefix and suffix. Then when calculating the size of the non-common part,
we end-up with a negative value which is reset to 0, thus the { = }.
In this example, the common prefix would be a/b/ and the common
suffix that does not overlap with the prefix part would be /c, so
I am imagining that a/b/{ = b}/c would be the desired output?
Yes, at least that's what I expected.
This is a really old thinko (dating back to June 2005). I'll queue
the patch on maint-1.7.6 (because 1.7.6.6 is slightly more than one
year old while 1.7.5.4 is a lot older) to allow distros that issue
incremental fixes on top of ancient versions of Git to pick up the
fix if they wanted to. Perhaps we would want to add a few tests?
I can easily understand why that was missed.
I will try to resubmit with tests very soon.
--
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] diff: Fix rename pretty-print when suffix and prefix overlap
In this example, the common prefix would be a/b/ and the common
suffix that does not overlap with the prefix part would be /c, so
I am imagining that a/b/{ = b}/c would be the desired output?
Yes, at least that's what I expected.
Surely it would be a/b/{b = }/c, that is, we have reduced the number of
b's by one. Or am I misunderstanding something?
(I'm guessing it was an all too obvious typo that was misread)
Indeed, read to fast and reproduced in suggested new message.
a/b/b/c = a/b/c is equivalent to a/b/{b = }/c
Thank you for proof-reading.
--
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] diff: Fix rename pretty-print when suffix and prefix overlap
When considering a rename for two files that have a suffix and a prefix
that can overlap, a confusing line is shown. As an example, renaming
a/b/b/c to a/b/c shows a/b/{ = }/b/c.
Currently, what we do is calculate the common prefix (a/b/), and the
common suffix (/b/c), but the same /b/ is actually counted both in
prefix and suffix. Then when calculating the size of the non-common part,
we end-up with a negative value which is reset to 0, thus the { = }.
Do not allow the common suffix to overlap the common prefix and stop
when reaching a / that would be in both.
Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
diff.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/diff.c b/diff.c
index 156fec4..80f4752 100644
--- a/diff.c
+++ b/diff.c
@@ -1290,7 +1290,16 @@ static char *pprint_rename(const char *a, const char *b)
old = a + len_a;
new = b + len_b;
sfx_length = 0;
- while (a = old b = new *old == *new) {
+ /*
+* Note:
+* if pfx_length is 0, old/new will never reach a - 1 because it
+* would mean the whole string is common suffix. But then, the
+* whole string would also be a common prefix, and we would not
+* have pfx_length equals 0.
+*/
+ while (a + pfx_length - 1 = old
+ b + pfx_length - 1 = new
+ *old == *new) {
if (*old == '/')
sfx_length = len_a - (old - a);
old--;
--
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] update-index: allow -h to also display options
Currently, when running git update-index -h, you only have usage displayed, but no options. That is not consistent with the behavior of other commands. It also means that the only way to display options is to use an unknown argument (or use the man page). Display usage with options when git update-index -h is invoked. Signed-off-by: Antoine Pelisse apeli...@gmail.com --- builtin/update-index.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/update-index.c b/builtin/update-index.c index ada1dff..3071ee6 100644 --- a/builtin/update-index.c +++ b/builtin/update-index.c @@ -796,7 +796,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix) }; if (argc == 2 !strcmp(argv[1], -h)) - usage(update_index_usage[0]); + usage_with_options(update_index_usage[0], options); git_config(git_default_config, NULL); -- 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] update-index: allow -h to also display options
diff --git a/builtin/update-index.c b/builtin/update-index.c index ada1dff..3071ee6 100644 --- a/builtin/update-index.c +++ b/builtin/update-index.c @@ -796,7 +796,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix) }; if (argc == 2 !strcmp(argv[1], -h)) - usage(update_index_usage[0]); + usage_with_options(update_index_usage[0], options); git_config(git_default_config, NULL); Ok I just realized that usage_with_options(update_index_usage, options); would be better... Antoine, -- 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/3] Make Git compile warning-free with Clang
Thanks John, I couldn't find any time to send that sum-up series. On Sun, Feb 3, 2013 at 3:37 PM, John Keeping j...@keeping.me.uk wrote: The first two patches here were sent to the list before but seem to have got lost in the noise [1][2]. The final one is new but was prompted by discussion in the same thread. -- 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] send-email: ignore files ending with ~
On Sun, Feb 3, 2013 at 3:55 PM, Alexandre Courbot gnu...@gmail.com wrote: It certainly happened to a lot of people already: you carefully prepare your set of patches, export them using format-patch --cover-letter, write your cover letter, and send the set like this: $ git send-email --to=somerenowneddeveloper --to=myfutureemployer --cc=thismailinglistiwanttoimpress 00* Why don't you use 00*.patch ? That seems dubious to me to ignore files specified on the command line. -- 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: Segmentation fault with latest git (070c57df)
In clean.c we have a string_list created on the stack with STRING_LIST_INIT_NODUP (there are probably others, I stopped at the first occurrence). But, STRING_LIST_INIT_NODUP doesn't init the list-cmp pointer which can thus be random. I don't have much time to provide a patch right now (have to go to work), but will tonight if still needed. Antoine, On Thu, Jan 31, 2013 at 7:49 AM, Jeff King p...@peff.net wrote: On Thu, Jan 31, 2013 at 01:35:21AM +, Jongman Heo wrote: Looks like following commit causes a segmentation fault in my machine (when running git pull or git fetch); commit 8dd5afc926acb9829ebf56e9b78826a5242cd638 Author: Junio C Hamano gits...@pobox.com Date: Mon Jan 7 12:24:55 2013 -0800 string-list: allow case-insensitive string list In my case, list-cmp (at get_entry_index() function) has an invalid address, obviously not an address of string comparision function, instead it points to 1. Can you show us a stack trace? The string-list functions are generic and get called in a lot of places. It would be useful to know which list is causing the problem. -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 -- 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: [RFC] git rm -u
I think `git add -u` would be closer. It would stage removal of files, but would not stage untracked files. It would stage other type of changes though. On Sat, Jan 19, 2013 at 10:47 PM, Tomas Carnecky tomas.carne...@gmail.com wrote: On Sat, 19 Jan 2013 16:35:18 -0500, Eric James Michael Ritz lobbyjo...@gmail.com wrote: Hello everyone, I am thinking about implementing a feature but I would appreciate any feedback before I begin, because more experienced Git developers and users may see some major problem that I do not. Earlier today I deleted a file from a repository. I deleted it normally, not by using `git rm`. So when I looked at `git status` on my terminal it told me about the file no longer being there. In my sleepy state of mind I ran `git rm -u` without thinking about. I did this because I have a habit of using `git add -u`. I know `git rm` does not support that option, but I tried it anyways without thinking about it. When I came to my senses and realized that does not work I began to wonder if `git rm -u` should exist. If any deleted, tracked files are not part of the index to commit then `git rm -u` would add that change to the index. This would save users the effort of having to type out `git rm filename`, and could be useful when a user is deleting multiple files. Does this sound like a reasonable, useful feature to Git? Or is there already a way to accomplish this which I have missed out of ignorance? Any thoughts and feedback would be greatly appreciated. Does `git add -A` do what you want? -- 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 -- 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/2] fix clang -Wtautological-compare with unsigned enum
BTW, I think it has been addressed [1] by clang already and that would explain why you don't have the warning when using clang trunk version. [1]: http://llvm-reviews.chandlerc.com/D113 Antoine, On Thu, Jan 17, 2013 at 5:44 PM, Linus Torvalds torva...@linux-foundation.org wrote: On Thu, Jan 17, 2013 at 3:00 AM, John Keeping j...@keeping.me.uk wrote: There's also a warning that triggers with clang 3.2 but not clang trunk, which I think is a legitimate warning - perhaps someone who understands integer type promotion better than me can explain why the code is OK (patch-score is declared as 'int'): builtin/apply.c:1044:47: warning: comparison of constant 18446744073709551615 with expression of type 'int' is always false [-Wtautological-constant-out-of-range-compare] if ((patch-score = strtoul(line, NULL, 10)) == ULONG_MAX) ^ ~ The warning seems to be very very wrong, and implies that clang has some nasty bug in it. Since patch-score is 'int', and UNLONG_MAX is 'unsigned long', the conversion rules for the comparison is that the int result from the assignment is cast to unsigned long. And if you cast (int)-1 to unsigned long, you *do* get ULONG_MAX. That's true regardless of whether long has the same number of bits as int or is bigger. The implicit cast will be done as a sign-extension (unsigned long is not signed, but the source type of 'int' *is* signed, and that is what determines the sign extension on casting). So the is always false is pure and utter crap. clang is wrong, and it is wrong in a way that implies that it actually generates incorrect code. It may well be worth making a clang bug report about this. That said, clang is certainly understandably confused. The code depends on subtle conversion rules and bit patterns, and is clearly very confusingly written. So it would probably be good to rewrite it as unsigned long val = strtoul(line, NULL, 10); if (val == ULONG_MAX) .. patch-score = val; instead. At which point you might as well make the comparison be = INT_MAX instead, since anything bigger than that is going to be bogus. So the git code is probably worth cleaning up, but for git it would be a cleanup. For clang, this implies a major bug and bad code generation. Linus Linus -- 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] fix some clang warnings
FWIW, I also happen to have the warning:
advice.c:69:2: warning: expression result unused [-Wunused-value]
error('%s' is not possible because you have unmerged files., me);
^~
./git-compat-util.h:314:55: note: expanded from:
#define error(fmt, ...) (error((fmt), ##__VA_ARGS__), -1)
^~
with clang: Ubuntu clang version 3.0-6ubuntu3 (tags/RELEASE_30/final)
(based on LLVM 3.0)
I can't say about other versions.
On Wed, Jan 16, 2013 at 5:53 PM, Junio C Hamano gits...@pobox.com wrote:
Jeff King p...@peff.net writes:
On Wed, Jan 16, 2013 at 03:53:23PM +0100, Max Horn wrote:
-#ifdef __GNUC__
+#if defined(__GNUC__) ! defined(__clang__)
#define config_error_nonbool(s) (config_error_nonbool(s), -1)
#endif
You don't say what the warning is, but I'm guessing it's complaining
about throwing away the return value from config_error_nonbool?
Yeah, I was wondering about the same thing. The other one looks
similar, ignoring the return value of error().
Also, is this some versions of clang do not like this? Or are all
versions of clang affected?
--
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
--
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] fix some clang warnings
Is it worth applying this at all, then? Or should we apply it but limit it with a clang version macro (they mention r163034, but I do not know if it is in a released version yet, nor what macros are available to inspect the version)? Please also note that building with clang is not warning-free (though I think it would be nice) -- 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] fix clang -Wconstant-conversion with bit fields
clang incorrectly reports a constant conversion warning (implicit
truncation to bit field) when using the flag = ~FLAG form, because
~FLAG needs to be truncated.
Convert this form to flag = flag ~FLAG fixes the issue as
the right operand now fits into the bit field.
Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
I'm sorry about this fix, it really seems bad, yet it's one step closer
to warning-free clang compilation.
It seems quite clear to me that it's a bug in clang.
bisect.c | 2 +-
builtin/checkout.c | 2 +-
builtin/reflog.c | 4 ++--
commit.c | 4 ++--
revision.c | 8
upload-pack.c | 4 ++--
6 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/bisect.c b/bisect.c
index bd1b7b5..34ac01d 100644
--- a/bisect.c
+++ b/bisect.c
@@ -63,7 +63,7 @@ static void clear_distance(struct commit_list *list)
{
while (list) {
struct commit *commit = list-item;
- commit-object.flags = ~COUNTED;
+ commit-object.flags = commit-object.flags ~COUNTED;
list = list-next;
}
}
diff --git a/builtin/checkout.c b/builtin/checkout.c
index a9c1b5a..2c83234 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -717,7 +717,7 @@ static void orphaned_commit_warning(struct commit *old,
struct commit *new)
init_revisions(revs, NULL);
setup_revisions(0, NULL, revs, NULL);
- object-flags = ~UNINTERESTING;
+ object-flags = object-flags ~UNINTERESTING;
add_pending_object(revs, object, sha1_to_hex(object-sha1));
for_each_ref(add_pending_uninteresting_ref, revs);
diff --git a/builtin/reflog.c b/builtin/reflog.c
index b3c9e27..3079c81 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -170,7 +170,7 @@ static int commit_is_complete(struct commit *commit)
}
/* clear flags from the objects we traversed */
for (i = 0; i found.nr; i++)
- found.objects[i].item-flags = ~STUDYING;
+ found.objects[i].item-flags = found.objects[i].item-flags
~STUDYING;
if (is_incomplete)
commit-object.flags |= INCOMPLETE;
else {
@@ -229,7 +229,7 @@ static void mark_reachable(struct expire_reflog_cb *cb)
struct commit_list *leftover = NULL;
for (pending = cb-mark_list; pending; pending = pending-next)
- pending-item-object.flags = ~REACHABLE;
+ pending-item-object.flags = pending-item-object.flags
~REACHABLE;
pending = cb-mark_list;
while (pending) {
diff --git a/commit.c b/commit.c
index e8eb0ae..800779d 100644
--- a/commit.c
+++ b/commit.c
@@ -883,7 +883,7 @@ struct commit_list *reduce_heads(struct commit_list *heads)
/* Uniquify */
for (p = heads; p; p = p-next)
- p-item-object.flags = ~STALE;
+ p-item-object.flags = p-item-object.flags ~STALE;
for (p = heads, num_head = 0; p; p = p-next) {
if (p-item-object.flags STALE)
continue;
@@ -894,7 +894,7 @@ struct commit_list *reduce_heads(struct commit_list *heads)
for (p = heads, i = 0; p; p = p-next) {
if (p-item-object.flags STALE) {
array[i++] = p-item;
- p-item-object.flags = ~STALE;
+ p-item-object.flags = p-item-object.flags ~STALE;
}
}
num_head = remove_redundant(array, num_head);
diff --git a/revision.c b/revision.c
index d7562ee..ed1c16d 100644
--- a/revision.c
+++ b/revision.c
@@ -787,9 +787,9 @@ static void limit_to_ancestry(struct commit_list *bottom,
struct commit_list *li
/* We are done with the TMP_MARK */
for (p = list; p; p = p-next)
- p-item-object.flags = ~TMP_MARK;
+ p-item-object.flags = p-item-object.flags ~TMP_MARK;
for (p = bottom; p; p = p-next)
- p-item-object.flags = ~TMP_MARK;
+ p-item-object.flags = p-item-object.flags ~TMP_MARK;
free_commit_list(rlist);
}
@@ -1948,7 +1948,7 @@ static int remove_duplicate_parents(struct commit *commit)
/* count them while clearing the temporary mark */
surviving_parents = 0;
for (p = commit-parents; p; p = p-next) {
- p-item-object.flags = ~TMP_MARK;
+ p-item-object.flags = p-item-object.flags ~TMP_MARK;
surviving_parents++;
}
return surviving_parents;
@@ -2378,7 +2378,7 @@ static struct commit *get_revision_1(struct rev_info
*revs)
if (revs-reflog_info) {
fake_reflog_parent(revs-reflog_info, commit);
- commit-object.flags = ~(ADDED | SEEN | SHOWN);
+ commit-object.flags = commit-object.flags ~(ADDED |
SEEN | SHOWN);
}
/*
diff --git a/upload-pack.c b/upload-pack.c
index 7c05b15..74d8f0e 100644
[PATCH 2/2] fix clang -Wtautological-compare with unsigned enum
Create a GREP_HEADER_FIELD_MIN so we can check that the field value is
sane and silent the clang warning.
Clang warning happens because the enum is unsigned (this is
implementation-defined, and there is no negative fields) and the check
is then tautological.
Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
I tried to consider discussion [1] and this [2] discussion on clang's list
With these two patches and the patch from Max Horne, I'm finally able to
compile with CC=clang CFLAGS=-Werror.
[1]: http://thread.gmane.org/gmane.comp.version-control.git/184908
[2]:
http://clang-developers.42468.n3.nabble.com/Possibly-invalid-enum-tautology-warning-td3233140.html
grep.c | 3 ++-
grep.h | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/grep.c b/grep.c
index 4bd1b8b..bb548ca 100644
--- a/grep.c
+++ b/grep.c
@@ -625,7 +625,8 @@ static struct grep_expr *prep_header_patterns(struct
grep_opt *opt)
for (p = opt-header_list; p; p = p-next) {
if (p-token != GREP_PATTERN_HEAD)
die(bug: a non-header pattern in grep header list.);
- if (p-field 0 || GREP_HEADER_FIELD_MAX = p-field)
+ if (p-field GREP_HEADER_FIELD_MIN ||
+ GREP_HEADER_FIELD_MAX = p-field)
die(bug: unknown header field %d, p-field);
compile_regexp(p, opt);
}
diff --git a/grep.h b/grep.h
index 8fc854f..e4a1df5 100644
--- a/grep.h
+++ b/grep.h
@@ -28,7 +28,8 @@ enum grep_context {
};
enum grep_header_field {
- GREP_HEADER_AUTHOR = 0,
+ GREP_HEADER_FIELD_MIN = 0,
+ GREP_HEADER_AUTHOR = GREP_HEADER_FIELD_MIN,
GREP_HEADER_COMMITTER,
GREP_HEADER_REFLOG,
--
1.8.1.1.435.g20d29be.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
Re: [PATCH 1/2] fix clang -Wconstant-conversion with bit fields
On Thu, Jan 17, 2013 at 12:08 AM, John Keeping j...@keeping.me.uk wrote: On Wed, Jan 16, 2013 at 11:47:22PM +0100, Antoine Pelisse wrote: clang incorrectly reports a constant conversion warning (implicit truncation to bit field) when using the flag = ~FLAG form, because ~FLAG needs to be truncated. Which version of clang did you see this with? I don't get these warnings with clang 3.2. Ubuntu clang version 3.0-6ubuntu3 (tags/RELEASE_30/final) (based on LLVM 3.0) It's good to know it's been fixed ! -- 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/2] fix clang -Wtautological-compare with unsigned enum
With these two patches and the patch from Max Horne, I'm deeply sorry for this typo Max -- 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 1/2] fix clang -Wconstant-conversion with bit fields
So I guess we should drop this patch, it's probably not worth it, especially if it's been fixed already by clang. On Thu, Jan 17, 2013 at 12:09 AM, Antoine Pelisse apeli...@gmail.com wrote: On Thu, Jan 17, 2013 at 12:08 AM, John Keeping j...@keeping.me.uk wrote: On Wed, Jan 16, 2013 at 11:47:22PM +0100, Antoine Pelisse wrote: clang incorrectly reports a constant conversion warning (implicit truncation to bit field) when using the flag = ~FLAG form, because ~FLAG needs to be truncated. Which version of clang did you see this with? I don't get these warnings with clang 3.2. Ubuntu clang version 3.0-6ubuntu3 (tags/RELEASE_30/final) (based on LLVM 3.0) It's good to know it's been fixed ! -- 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] am: invoke perl's strftime in C locale
This puts all of perl into the C locale, which would mean error messages from perl would be in English rather than the user's language. It probably isn't a big deal, because that snippet of perl is short and not likely to produce problems, but I wonder how hard it would be to set the locale just for the strftime call. Maybe just setting LC_TIME to C would do ... From locale(7) man page: LC_TIME changes the behavior of the strftime(3) function to display the current time in a locally acceptable form; for example, most of Europe uses a 24-hour clock versus the 12-hour clock used in the United States. -- 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: git send-email should not allow 'y' for in-reply-to
On Fri, Jan 11, 2013 at 10:23 PM, Jeff King p...@peff.net wrote: On Fri, Jan 11, 2013 at 08:13:57PM +, Matt Seitz (matseitz) wrote: How about What Message-ID to use as In-Reply-To for the first email? or Provide the Message-ID to use as In-Reply-To for the first email:. seem fine to me. Maybe somebody who has been confused by it can offer more. At any rate, patches welcome. Suggestion: Message-ID to use as In-Reply-To for the first email:. Simple and unlikely to generate a y or n response. Putting Message-ID first makes it more obvious what data is being asked for by this prompt. You'd think. But the existing message that has been causing problems is: Message-ID to be used as In-Reply-To for the first email? which is more or less what you are proposing. I do think a colon rather than a question mark helps indicate that the response is not yes/no. That is true. I'm definitely not a wording person, but assuming people who make the mistake probably don't read the whole sentence out of laziness (that might be somehow extreme though ;), starting it with what makes it obvious at first sight that you can't answer yes/no. That is not true if the message starts with Message-ID .. which doesn't look like a question. Now it feels like you have agree or not. Antoine, -- 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: git send-email should not allow 'y' for in-reply-to
On Fri, Jan 11, 2013 at 11:18 PM, Junio C Hamano gits...@pobox.com wrote: The exchange, when you do not have a configuration, goes like this: $ git send-email 0001-filename-of-the-patch.patch 0001-filename-of-the-patch.patch Who should the emails be sent to (if any)? junio Are you sure you want to use junio [y/N]? y Message-ID to be used as In-Reply-To for the first email (if any)? Why not do this instead? $ git send-email 0001-filename-of-the-patch.patch 0001-filename-of-the-patch.patch Who should the emails be sent to (if any)? junio Are you sure you want to use junio [y/N]? y Is this a response to an existing message [y/N]? y I'm not sure about the extra question. If the user doesn't care, he will probably use the empty default, which will result in the same number of steps. If the user cares, he probably knows what he's doing and will give a sensible value. What is the Message-ID of the message you are replying to? I would simply go for: What Message-ID are you replying to (if any)? If I don't know what to answer, I would definitely not say y/yes/n/no, but press enter directly. -- 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 v2 03/10] mailmap: remove email copy and length limitation
+static struct string_list_item *lookup_prefix(struct string_list *map,
+ const char *string, size_t len)
+{
+ int i = string_list_find_insert_index(map, string, 1);
+ if (i 0) {
+ /* exact match */
+ i = -1 - i;
+ /* does it match exactly? */
+ if (!map-items[i].string[len])
+ return map-items[i];
I'm not sure the condition above is necessary, as I don't see why an
exact match would not be an exact match.
We have to trust the cmp function (that mailmap sets itself) to not
return 0 when the lengths are different.
+ }
+
+ /*
+* i is at the exact match to an overlong key, or
+* location the possibly overlong key would be inserted,
+* which must be after the real location of the key.
+*/
+ while (0 = --i i map-nr) {
+ int cmp = strncasecmp(map-items[i].string, string, len);
+ if (cmp 0)
+ /*
+* i points at a key definitely below the prefix;
+* the map does not have string[0:len] in it.
+*/
+ break;
+ else if (!cmp !map-items[i].string[len])
+ /* found it */
+ return map-items[i];
+ /*
+* otherwise, the string at i may be string[0:len]
+* followed by a string that sorts later than string[len:];
+* keep trying.
+*/
+ }
+ return NULL;
+}
+
I've tried to think about nasty use cases but everything seems fine.
--
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 v2] status: always report ignored tracked directories
Here is my attempt... When enumerating paths that are ignored, paths the index knows about are not included in the result. The index knows about check is done by consulting the name hash, not the actual contents of the index: - When core.ignorecase is false, directory names are not in the name hash, and ignored ones are shown as ignored (directories can never be tracked anyway). - When core.ignorecase is true, however, the name hash keeps track of the names of directories, in order to detect additions of the paths under different cases. This causes ignored directories to be mistakenly excluded when enumerating ignored paths. Stop excluding directories that are in the name hash when looking for ignored files in dir_add_name(); the names that are actually in the index are excluded much earlier in the callchain in treat_file(), so this fix will not make them mistakenly identified as ignored. Signed-off-by: Antoine Pelisse apeli...@gmail.com Reviewed-by: Jeff King p...@peff.net Signed-off-by: Junio C Hamano gits...@pobox.com I like it a lot, thanks to both of you -- 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 v2] status: always report ignored tracked directories
Tracked directories (i.e. directories containing tracked files) that
are ignored must be reported as ignored if they contain untracked files.
Currently, files in the index can't be reported as ignored and are
automatically dropped from the list:
- When core.ignorecase is false, directories (which are not directly
tracked) are not listed as part of the index, and the directory can be
shown as ignored.
- When core.ignorecase is true on the other hand, directories are
reported as part of the index, and the directory is dropped, thus not
displayed as ignored.
Fix that behavior by allowing indexed file to be added when looking for
ignored files.
- Ignored tracked and untracked directories are treated the same way
when looking for ignored files, so we should not care about their index
status.
- Files are dismissed by treat_file() if they belong to the
index, so that step would have been a no-op anyway.
Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
dir.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/dir.c b/dir.c
index 9b80348..f836590 100644
--- a/dir.c
+++ b/dir.c
@@ -672,7 +672,8 @@ static struct dir_entry *dir_entry_new(const char
*pathname, int len)
static struct dir_entry *dir_add_name(struct dir_struct *dir, const char
*pathname, int len)
{
- if (cache_name_exists(pathname, len, ignore_case))
+ if (!(dir-flags DIR_SHOW_IGNORED)
+ cache_name_exists(pathname, len, ignore_case))
return NULL;
ALLOC_GROW(dir-entries, dir-nr+1, dir-alloc);
@@ -877,11 +878,7 @@ static int treat_file(struct dir_struct *dir, struct
strbuf *path, int exclude,
if (exclude)
exclude_file = !(dir-flags DIR_SHOW_IGNORED);
else if (dir-flags DIR_SHOW_IGNORED) {
- /*
-* Optimization:
-* Don't spend time on indexed files, they won't be
-* added to the list anyway
-*/
+ /* Always exclude indexed files */
struct cache_entry *ce = index_name_exists(the_index,
path-buf, path-len, ignore_case);
--
1.7.12.4.3.g90f5e2d
--
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] status: report ignored yet tracked directories
Tracked directories (i.e. directories containing tracked files) that
are ignored must be reported as ignored if they contain untracked files.
Currently, tracked files or directories can't be reported untracked or ignored.
Remove that constraint when searching ignored files.
Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
Torsten, Jeff,
Can you please test this patch and tell me if this is better ? (t7061 is now
successful with core.ignorecase=true)
This patch applies on top of ap/status-ignored-in-ignored-directory (but
should also apply cleanly on top of next for testing purpose).
Thanks,
dir.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/dir.c b/dir.c
index 9b80348..eefa8ab 100644
--- a/dir.c
+++ b/dir.c
@@ -672,7 +672,8 @@ static struct dir_entry *dir_entry_new(const char
*pathname, int len)
static struct dir_entry *dir_add_name(struct dir_struct *dir, const char
*pathname, int len)
{
- if (cache_name_exists(pathname, len, ignore_case))
+ if (!(dir-flags DIR_SHOW_IGNORED)
+ cache_name_exists(pathname, len, ignore_case))
return NULL;
ALLOC_GROW(dir-entries, dir-nr+1, dir-alloc);
--
1.7.12.4.2.geb8c5b8.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 00/10] Log mailmap optimization
Hi, Here is a reroll of ap/log-mailmap. The idea is to use another preparatory step: Allow case-insensitive and length search in list_lookup We can now search for mapping name and email without any copy. Of course a copy is then necessary to store the info, but we no longer need any copy to look-up the mapping (useful for replacing or not before grep). Thanks, Antoine Pelisse (10): list_lookup: create case and length search Use split_ident_line to parse author and committer mailmap: remove email copy and length limitation mailmap: simplify map_user() interface mailmap: add mailmap structure to rev_info and pp pretty: use mailmap to display username and email log: add --use-mailmap option test: add test for --use-mailmap option log: grep author/committer using mailmap log: add log.mailmap configuration option Documentation/config.txt | 4 + Documentation/git-log.txt | 5 ++ builtin/blame.c | 183 ++ builtin/log.c | 16 +++- builtin/shortlog.c| 54 -- commit.h | 1 + log-tree.c| 1 + mailmap.c | 55 +- mailmap.h | 4 +- pretty.c | 114 - revision.c| 54 ++ revision.h| 1 + string-list.c | 30 ++-- string-list.h | 2 + t/t4203-mailmap.sh| 56 ++ 15 files changed, 349 insertions(+), 231 deletions(-) -- 1.7.12.4.3.g2036a08.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 01/10] list_lookup: create case and length search
Create a new function to look-up a string in a string_list, but:
- add a new parameter to ignore case differences
- add a length parameter to search for a substring
The idea is to avoid several copies (lowering a string before searching
it when we just want to ignore case), or copying a substring of a bigger
string to search it in the string_list
Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
string-list.c | 30 --
string-list.h | 2 ++
2 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/string-list.c b/string-list.c
index 397e6cf..f06e110 100644
--- a/string-list.c
+++ b/string-list.c
@@ -4,13 +4,21 @@
/* if there is no exact match, point to the index where the entry could be
* inserted */
static int get_entry_index(const struct string_list *list, const char *string,
- int *exact_match)
+ int *exact_match, int case_sensitive, size_t n)
{
int left = -1, right = list-nr;
while (left + 1 right) {
+ int compare;
int middle = (left + right) / 2;
- int compare = strcmp(string, list-items[middle].string);
+ if (case_sensitive)
+ compare = strncmp(string, list-items[middle].string,
n);
+ else
+ compare = strncasecmp(string,
list-items[middle].string, n);
+ /* Make sure our string is not a substring of item string */
+ if (!compare n != -1)
+ if (list-items[middle].string[n] != '\0')
+ compare = -1;
if (compare 0)
right = middle;
else if (compare 0)
@@ -29,7 +37,7 @@ static int get_entry_index(const struct string_list *list,
const char *string,
static int add_entry(int insert_at, struct string_list *list, const char
*string)
{
int exact_match = 0;
- int index = insert_at != -1 ? insert_at : get_entry_index(list, string,
exact_match);
+ int index = insert_at != -1 ? insert_at : get_entry_index(list, string,
exact_match, 1, -1);
if (exact_match)
return -1 - index;
@@ -70,7 +78,7 @@ struct string_list_item *string_list_insert_at_index(struct
string_list *list,
int string_list_has_string(const struct string_list *list, const char *string)
{
int exact_match;
- get_entry_index(list, string, exact_match);
+ get_entry_index(list, string, exact_match, 1, -1);
return exact_match;
}
@@ -78,7 +86,7 @@ int string_list_find_insert_index(const struct string_list
*list, const char *st
int negative_existing_index)
{
int exact_match;
- int index = get_entry_index(list, string, exact_match);
+ int index = get_entry_index(list, string, exact_match, 1, -1);
if (exact_match)
index = -1 - (negative_existing_index ? index : 0);
return index;
@@ -86,7 +94,17 @@ int string_list_find_insert_index(const struct string_list
*list, const char *st
struct string_list_item *string_list_lookup(struct string_list *list, const
char *string)
{
- int exact_match, i = get_entry_index(list, string, exact_match);
+ int exact_match, i = get_entry_index(list, string, exact_match, 1, -1);
+ if (!exact_match)
+ return NULL;
+ return list-items + i;
+}
+
+struct string_list_item *string_list_lookup_extended(struct string_list *list,
+const char *string, int case_sensitive, size_t n)
+{
+ int exact_match, i = get_entry_index(list, string, exact_match,
+case_sensitive, n);
if (!exact_match)
return NULL;
return list-items + i;
diff --git a/string-list.h b/string-list.h
index c50b0d0..4f5ae19 100644
--- a/string-list.h
+++ b/string-list.h
@@ -62,6 +62,8 @@ struct string_list_item *string_list_insert(struct
string_list *list, const char
struct string_list_item *string_list_insert_at_index(struct string_list *list,
int insert_at, const char
*string);
struct string_list_item *string_list_lookup(struct string_list *list, const
char *string);
+struct string_list_item *string_list_lookup_extended(struct string_list *list,
+const char *string, int case_sensitive, size_t n);
/*
* Remove all but the first of consecutive entries with the same
--
1.7.12.4.3.g2036a08.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 02/10] Use split_ident_line to parse author and committer
Currently blame.c::get_acline, pretty.c::pp_user_info() and
shortlog.c::insert_one_record are parsing author name, email, time and
tz themselves.
Use ident.c::split_ident_line for better code reuse.
Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
builtin/blame.c| 59 +++---
builtin/shortlog.c | 36 +
pretty.c | 35 +++-
3 files changed, 52 insertions(+), 78 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index cfae569..dd4aff9 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1343,8 +1343,9 @@ static void get_ac_line(const char *inbuf, const char
*what,
int mail_len, char *mail,
unsigned long *time, const char **tz)
{
- int len, tzlen, maillen;
- char *tmp, *endp, *timepos, *mailpos;
+ struct ident_split ident;
+ int len, tzlen, maillen, namelen;
+ char *tmp, *endp, *mailpos;
tmp = strstr(inbuf, what);
if (!tmp)
@@ -1355,7 +1356,10 @@ static void get_ac_line(const char *inbuf, const char
*what,
len = strlen(tmp);
else
len = endp - tmp;
- if (person_len = len) {
+ if (person_len = len)
+ goto error_out;
+
+ if (split_ident_line(ident, tmp, len)) {
error_out:
/* Ugh */
*tz = (unknown);
@@ -1364,47 +1368,26 @@ static void get_ac_line(const char *inbuf, const char
*what,
*time = 0;
return;
}
- memcpy(person, tmp, len);
- tmp = person;
- tmp += len;
- *tmp = 0;
- while (person tmp *tmp != ' ')
- tmp--;
- if (tmp = person)
- goto error_out;
- *tz = tmp+1;
- tzlen = (person+len)-(tmp+1);
+ namelen = ident.name_end - ident.name_begin;
+ memcpy(person, ident.name_begin, namelen);
+ person[namelen] = 0;
- *tmp = 0;
- while (person tmp *tmp != ' ')
- tmp--;
- if (tmp = person)
- goto error_out;
- *time = strtoul(tmp, NULL, 10);
- timepos = tmp;
+ maillen = ident.mail_end - ident.mail_begin + 2;
+ memcpy(mail, ident.mail_begin - 1, maillen);
+ mail[maillen] = 0;
- *tmp = 0;
- while (person tmp !(*tmp == ' ' tmp[1] == ''))
- tmp--;
- if (tmp = person)
- return;
- mailpos = tmp + 1;
- *tmp = 0;
- maillen = timepos - tmp;
- memcpy(mail, mailpos, maillen);
+ *time = strtoul(ident.date_begin, NULL, 10);
- if (!mailmap.nr)
- return;
+ tzlen = ident.tz_end - ident.tz_begin;
- /*
-* mailmap expansion may make the name longer.
-* make room by pushing stuff down.
-*/
- tmp = person + person_len - (tzlen + 1);
- memmove(tmp, *tz, tzlen);
+ /* Place tz at the end of person */
+ *tz = tmp = person + person_len - (tzlen + 1);
+ memcpy(tmp, ident.tz_begin, tzlen);
tmp[tzlen] = 0;
- *tz = tmp;
+
+ if (!mailmap.nr)
+ return;
/*
* Now, convert both name and e-mail using mailmap
diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index b316cf3..03c6cd7 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -40,40 +40,24 @@ static void insert_one_record(struct shortlog *log,
char emailbuf[1024];
size_t len;
const char *eol;
- const char *boemail, *eoemail;
struct strbuf subject = STRBUF_INIT;
+ struct ident_split ident;
- boemail = strchr(author, '');
- if (!boemail)
- return;
- eoemail = strchr(boemail, '');
- if (!eoemail)
+ if (split_ident_line(ident, author, strlen(author)))
return;
/* copy author name to namebuf, to support matching on both name and
email */
- memcpy(namebuf, author, boemail - author);
- len = boemail - author;
- while (len 0 isspace(namebuf[len-1]))
- len--;
+ len = ident.name_end - ident.name_begin;
+ memcpy(namebuf, ident.name_begin, len);
namebuf[len] = 0;
/* copy email name to emailbuf, to allow email replacement as well */
- memcpy(emailbuf, boemail+1, eoemail - boemail);
- emailbuf[eoemail - boemail - 1] = 0;
-
- if (!map_user(log-mailmap, emailbuf, sizeof(emailbuf), namebuf,
sizeof(namebuf))) {
- while (author boemail isspace(*author))
- author++;
- for (len = 0;
-len sizeof(namebuf) - 1 author + len boemail;
-len++)
- namebuf[len] = author[len];
- while (0 len isspace(namebuf[len-1]))
- len--;
- namebuf[len] = '\0';
- }
- else
[PATCH 05/10] mailmap: add mailmap structure to rev_info and pp
the mailmap string_list structure filled with mailmap
information is passed along from rev_info to pretty_print_context
to provide mailmap information to pretty print each commits
with the correct username and email.
Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
commit.h | 1 +
log-tree.c | 1 +
revision.h | 1 +
3 files changed, 3 insertions(+)
diff --git a/commit.h b/commit.h
index b6ad8f3..7f8f987 100644
--- a/commit.h
+++ b/commit.h
@@ -89,6 +89,7 @@ struct pretty_print_context {
char *notes_message;
struct reflog_walk_info *reflog_info;
const char *output_encoding;
+ struct string_list *mailmap;
};
struct userformat_want {
diff --git a/log-tree.c b/log-tree.c
index 4f86def..92254fd 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -671,6 +671,7 @@ void show_log(struct rev_info *opt)
ctx.preserve_subject = opt-preserve_subject;
ctx.reflog_info = opt-reflog_info;
ctx.fmt = opt-commit_format;
+ ctx.mailmap = opt-mailmap;
pretty_print_commit(ctx, commit, msgbuf);
if (opt-add_signoff)
diff --git a/revision.h b/revision.h
index 059bfff..83a79f5 100644
--- a/revision.h
+++ b/revision.h
@@ -143,6 +143,7 @@ struct rev_info {
const char *subject_prefix;
int no_inline;
int show_log_size;
+ struct string_list *mailmap;
/* Filter by commit log message */
struct grep_opt grep_filter;
--
1.7.12.4.3.g2036a08.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 08/10] test: add test for --use-mailmap option
The new option '--use-mailmap' can be used to make sure that mailmap file is used to convert name when running log commands. The test is simple and checks that the Author line is correctly replaced when running log. Signed-off-by: Antoine Pelisse apeli...@gmail.com --- t/t4203-mailmap.sh | 14 ++ 1 file changed, 14 insertions(+) diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh index 1f182f6..db043dc 100755 --- a/t/t4203-mailmap.sh +++ b/t/t4203-mailmap.sh @@ -239,6 +239,20 @@ test_expect_success 'Log output (complex mapping)' ' test_cmp expect actual ' +cat expect \EOF +Author: CTO c...@company.xx +Author: Santa Claus santa.cl...@northpole.xx +Author: Santa Claus santa.cl...@northpole.xx +Author: Other Author ot...@author.xx +Author: Other Author ot...@author.xx +Author: Some Dude s...@dude.xx +Author: A U Thor aut...@example.com +EOF +test_expect_success 'Log output with --use-mailmap' ' + git log --use-mailmap | grep Author actual + test_cmp expect actual +' + # git blame cat expect \EOF ^OBJI (A U Thor DATE 1) one -- 1.7.12.4.3.g2036a08.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 04/10] mailmap: simplify map_user() interface
The new string_list_lookup_extended() allows us to simplify map_user(),
mostly to avoid copies of string buffers. It also simplifies caller
functions.
map_user() directly receive pointers and length from the commit buffer
as mail and name. If mapping of the user and mail can be done, the
pointer is updated to a new location. Lengths are also updated if
necessary.
The caller of map_user() can then copy the new email and name if
necessary.
Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
builtin/blame.c| 156 +++--
builtin/shortlog.c | 32 +--
mailmap.c | 43 +++
mailmap.h | 4 +-
pretty.c | 35 +---
5 files changed, 126 insertions(+), 144 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index dd4aff9..86450e3 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1321,31 +1321,31 @@ static void pass_blame(struct scoreboard *sb, struct
origin *origin, int opt)
* Information on commits, used for output.
*/
struct commit_info {
- const char *author;
- const char *author_mail;
+ struct strbuf author;
+ struct strbuf author_mail;
unsigned long author_time;
- const char *author_tz;
+ struct strbuf author_tz;
/* filled only when asked for details */
- const char *committer;
- const char *committer_mail;
+ struct strbuf committer;
+ struct strbuf committer_mail;
unsigned long committer_time;
- const char *committer_tz;
+ struct strbuf committer_tz;
- const char *summary;
+ struct strbuf summary;
};
/*
* Parse author/committer line in the commit object buffer
*/
static void get_ac_line(const char *inbuf, const char *what,
- int person_len, char *person,
- int mail_len, char *mail,
- unsigned long *time, const char **tz)
+ struct strbuf *name, struct strbuf *mail,
+ unsigned long *time, struct strbuf *tz)
{
struct ident_split ident;
- int len, tzlen, maillen, namelen;
- char *tmp, *endp, *mailpos;
+ size_t len, maillen, namelen;
+ char *tmp, *endp;
+ const char *tmpname, *tmpmail;
tmp = strstr(inbuf, what);
if (!tmp)
@@ -1356,51 +1356,61 @@ static void get_ac_line(const char *inbuf, const char
*what,
len = strlen(tmp);
else
len = endp - tmp;
- if (person_len = len)
- goto error_out;
if (split_ident_line(ident, tmp, len)) {
error_out:
/* Ugh */
- *tz = (unknown);
- strcpy(person, *tz);
- strcpy(mail, *tz);
+ tmp = (unknown);
+ strbuf_addstr(name, tmp);
+ strbuf_addstr(mail, tmp);
+ strbuf_addstr(tz, tmp);
*time = 0;
return;
}
namelen = ident.name_end - ident.name_begin;
- memcpy(person, ident.name_begin, namelen);
- person[namelen] = 0;
+ tmpname = ident.name_begin;
- maillen = ident.mail_end - ident.mail_begin + 2;
- memcpy(mail, ident.mail_begin - 1, maillen);
- mail[maillen] = 0;
+ maillen = ident.mail_end - ident.mail_begin;
+ tmpmail = ident.mail_begin;
*time = strtoul(ident.date_begin, NULL, 10);
- tzlen = ident.tz_end - ident.tz_begin;
-
- /* Place tz at the end of person */
- *tz = tmp = person + person_len - (tzlen + 1);
- memcpy(tmp, ident.tz_begin, tzlen);
- tmp[tzlen] = 0;
-
- if (!mailmap.nr)
- return;
+ len = ident.tz_end - ident.tz_begin;
+ strbuf_add(tz, ident.tz_begin, len);
/*
* Now, convert both name and e-mail using mailmap
*/
- if (map_user(mailmap, mail+1, mail_len-1, person, tmp-person-1)) {
- /* Add a trailing '' to email, since map_user returns plain
emails
- Note: It already has '', since we replace from mail+1 */
- mailpos = memchr(mail, '\0', mail_len);
- if (mailpos mailpos-mail mail_len - 1) {
- *mailpos = '';
- *(mailpos+1) = '\0';
- }
- }
+ map_user(mailmap, tmpmail, maillen,
+tmpname, namelen);
+
+ strbuf_addf(mail, %.*s, (int)maillen, tmpmail);
+ strbuf_add(name, tmpname, namelen);
+}
+
+static void commit_info_init(struct commit_info *ci)
+{
+
+ strbuf_init(ci-author, 0);
+ strbuf_init(ci-author_mail, 0);
+ strbuf_init(ci-author_tz, 0);
+ strbuf_init(ci-committer, 0);
+ strbuf_init(ci-committer_mail, 0);
+ strbuf_init(ci-committer_tz, 0);
+ strbuf_init(ci-summary, 0);
+}
+
+static void commit_info_destroy(struct commit_info *ci)
+{
+
+ strbuf_release(ci-author);
+ strbuf_release(ci
[PATCH 10/10] log: add log.mailmap configuration option
Teach log.mailmap configuration variable to turn --use-mailmap
option on to git log, git show and git whatchanged.
The --no-use-mailmap option from the command line can countermand
the setting.
Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
Documentation/config.txt | 4
builtin/log.c| 7 +++
t/t4203-mailmap.sh | 24
3 files changed, 35 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index bf8f911..226362a 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1509,6 +1509,10 @@ log.showroot::
Tools like linkgit:git-log[1] or linkgit:git-whatchanged[1], which
normally hide the root commit will now show it. True by default.
+log.mailmap::
+ If true, makes linkgit:git-log[1], linkgit:git-show[1], and
+ linkgit:git-whatchanged[1] assume `--use-mailmap`.
+
mailmap.file::
The location of an augmenting mailmap file. The default
mailmap, located in the root of the repository, is loaded
diff --git a/builtin/log.c b/builtin/log.c
index d2bd8ce..16e6520 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -31,6 +31,7 @@ static int default_abbrev_commit;
static int default_show_root = 1;
static int decoration_style;
static int decoration_given;
+static int use_mailmap_config;
static const char *fmt_patch_subject_prefix = PATCH;
static const char *fmt_pretty;
@@ -106,6 +107,7 @@ static void cmd_log_init_finish(int argc, const char
**argv, const char *prefix,
OPT_END()
};
+ mailmap = use_mailmap_config;
argc = parse_options(argc, argv, prefix,
builtin_log_options, builtin_log_usage,
PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |
@@ -358,6 +360,11 @@ static int git_log_config(const char *var, const char
*value, void *cb)
}
if (!prefixcmp(var, color.decorate.))
return parse_decorate_color_config(var, 15, value);
+ if (!strcmp(var, log.mailmap)) {
+ use_mailmap_config = git_config_bool(var, value);
+ return 0;
+ }
+
if (grep_config(var, value, cb) 0)
return -1;
return git_diff_ui_config(var, value, cb);
diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index e16187f..7d4d31c 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -255,6 +255,21 @@ test_expect_success 'Log output with --use-mailmap' '
'
cat expect \EOF
+Author: CTO c...@company.xx
+Author: Santa Claus santa.cl...@northpole.xx
+Author: Santa Claus santa.cl...@northpole.xx
+Author: Other Author ot...@author.xx
+Author: Other Author ot...@author.xx
+Author: Some Dude s...@dude.xx
+Author: A U Thor aut...@example.com
+EOF
+
+test_expect_success 'Log output with log.mailmap' '
+ git -c log.mailmap=True log | grep Author actual
+ test_cmp expect actual
+'
+
+cat expect \EOF
Author: Santa Claus santa.cl...@northpole.xx
Author: Santa Claus santa.cl...@northpole.xx
EOF
@@ -263,6 +278,15 @@ test_expect_success 'Grep author with --use-mailmap' '
git log --use-mailmap --author Santa | grep Author actual
test_cmp expect actual
'
+cat expect \EOF
+Author: Santa Claus santa.cl...@northpole.xx
+Author: Santa Claus santa.cl...@northpole.xx
+EOF
+
+test_expect_success 'Grep author with log.mailmap' '
+ git -c log.mailmap=True log --author Santa | grep Author actual
+ test_cmp expect actual
+'
expect
--
1.7.12.4.3.g2036a08.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 03/10] mailmap: remove email copy and length limitation
In map_user(), the new string_list_lookup_extended() allows us to remove
the copy of email buffer to lower it.
This also removes the limitation on the length of the copy buffer and
simplifies the function.
Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
mailmap.c | 22 ++
1 file changed, 6 insertions(+), 16 deletions(-)
diff --git a/mailmap.c b/mailmap.c
index ea4b471..9db8a1b 100644
--- a/mailmap.c
+++ b/mailmap.c
@@ -193,8 +193,7 @@ int map_user(struct string_list *map,
char *end_of_email;
struct string_list_item *item;
struct mailmap_entry *me;
- char buf[1024], *mailbuf;
- int i;
+ size_t maillen;
/* figure out space requirement for email */
end_of_email = strchr(email, '');
@@ -204,18 +203,11 @@ int map_user(struct string_list *map,
if (!end_of_email)
return 0;
}
- if (end_of_email - email + 1 sizeof(buf))
- mailbuf = buf;
- else
- mailbuf = xmalloc(end_of_email - email + 1);
-
- /* downcase the email address */
- for (i = 0; i end_of_email - email; i++)
- mailbuf[i] = tolower(email[i]);
- mailbuf[i] = 0;
-
- debug_mm(map_user: map '%s' %s\n, name, mailbuf);
- item = string_list_lookup(map, mailbuf);
+
+ maillen = end_of_email - email;
+
+ debug_mm(map_user: map '%s' %.*s\n, name, maillen, email);
+ item = string_list_lookup_extended(map, email, 0, maillen);
if (item != NULL) {
me = (struct mailmap_entry *)item-util;
if (me-namemap.nr) {
@@ -226,8 +218,6 @@ int map_user(struct string_list *map,
item = subitem;
}
}
- if (mailbuf != buf)
- free(mailbuf);
if (item != NULL) {
struct mailmap_info *mi = (struct mailmap_info *)item-util;
if (mi-name == NULL (mi-email == NULL || maxlen_email ==
0)) {
--
1.7.12.4.3.g2036a08.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 09/10] log: grep author/committer using mailmap
Currently you can use mailmap to display log authors and committers
but you can't use the mailmap to find commits with mapped values.
This commit allows you to run:
git log --use-mailmap --author mapped_name_or_email
git log --use-mailmap --committer mapped_name_or_email
Of course it only works if the --use-mailmap option is used.
The new name and email are copied only when necessary.
Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
revision.c | 54 ++
t/t4203-mailmap.sh | 18 ++
2 files changed, 72 insertions(+)
diff --git a/revision.c b/revision.c
index 95d21e6..2cce85a 100644
--- a/revision.c
+++ b/revision.c
@@ -13,6 +13,7 @@
#include decorate.h
#include log-tree.h
#include string-list.h
+#include mailmap.h
volatile show_early_output_fn_t show_early_output;
@@ -2219,6 +2220,51 @@ static int rewrite_parents(struct rev_info *revs, struct
commit *commit)
return 0;
}
+static int commit_rewrite_person(struct strbuf *buf, const char *what, struct
string_list *mailmap)
+{
+ char *person, *endp;
+ size_t len, namelen, maillen;
+ const char *name;
+ const char *mail;
+ struct ident_split ident;
+
+ person = strstr(buf-buf, what);
+ if (!person)
+ return 0;
+
+ person += strlen(what);
+ endp = strchr(person, '\n');
+ if (!endp)
+ return 0;
+
+ len = endp - person;
+
+ if (split_ident_line(ident, person, len))
+ return 0;
+
+ mail = ident.mail_begin;
+ maillen = ident.mail_end - ident.mail_begin;
+ name = ident.name_begin;
+ namelen = ident.name_end - ident.name_begin;
+
+ if (map_user(mailmap, mail, maillen, name, namelen)) {
+ struct strbuf namemail = STRBUF_INIT;
+
+ strbuf_addf(namemail, %.*s %.*s,
+ (int)namelen, name, (int)maillen, mail);
+
+ strbuf_splice(buf, ident.name_begin - buf-buf,
+ ident.mail_end - ident.name_begin + 1,
+ namemail.buf, namemail.len);
+
+ strbuf_release(namemail);
+
+ return 1;
+ }
+
+ return 0;
+}
+
static int commit_match(struct commit *commit, struct rev_info *opt)
{
int retval;
@@ -2237,6 +2283,14 @@ static int commit_match(struct commit *commit, struct
rev_info *opt)
if (buf.len)
strbuf_addstr(buf, commit-buffer);
+ if (opt-mailmap) {
+ if (!buf.len)
+ strbuf_addstr(buf, commit-buffer);
+
+ commit_rewrite_person(buf, \nauthor , opt-mailmap);
+ commit_rewrite_person(buf, \ncommitter , opt-mailmap);
+ }
+
/* Append fake message parts as needed */
if (opt-show_notes) {
if (!buf.len)
diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index db043dc..e16187f 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -248,11 +248,29 @@ Author: Other Author ot...@author.xx
Author: Some Dude s...@dude.xx
Author: A U Thor aut...@example.com
EOF
+
test_expect_success 'Log output with --use-mailmap' '
git log --use-mailmap | grep Author actual
test_cmp expect actual
'
+cat expect \EOF
+Author: Santa Claus santa.cl...@northpole.xx
+Author: Santa Claus santa.cl...@northpole.xx
+EOF
+
+test_expect_success 'Grep author with --use-mailmap' '
+ git log --use-mailmap --author Santa | grep Author actual
+ test_cmp expect actual
+'
+
+expect
+
+test_expect_success 'Only grep replaced author with --use-mailmap' '
+ git log --use-mailmap --author c...@coompany.xx actual
+ test_cmp expect actual
+'
+
# git blame
cat expect \EOF
^OBJI (A U Thor DATE 1) one
--
1.7.12.4.3.g2036a08.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 07/10] log: add --use-mailmap option
Add the --use-mailmap option to log commands. It allows
to display names from mailmap file when displaying logs,
whatever the format used.
Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
Documentation/git-log.txt | 5 +
builtin/log.c | 9 -
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 585dac4..a99be97 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -47,6 +47,11 @@ OPTIONS
Print out the ref name given on the command line by which each
commit was reached.
+--use-mailmap::
+ Use mailmap file to map author and committer names and email
+ to canonical real names and email addresses. See
+ linkgit:git-shortlog[1].
+
--full-diff::
Without this flag, git log -p path... shows commits that
touch the specified paths, and diffs about the same specified
diff --git a/builtin/log.c b/builtin/log.c
index e7b7db1..d2bd8ce 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -22,6 +22,7 @@
#include branch.h
#include streaming.h
#include version.h
+#include mailmap.h
/* Set a default date-time format for git log (log.date config variable) */
static const char *default_date_mode = NULL;
@@ -94,11 +95,12 @@ static void cmd_log_init_finish(int argc, const char
**argv, const char *prefix,
struct rev_info *rev, struct setup_revision_opt *opt)
{
struct userformat_want w;
- int quiet = 0, source = 0;
+ int quiet = 0, source = 0, mailmap = 0;
const struct option builtin_log_options[] = {
OPT_BOOLEAN(0, quiet, quiet, N_(suppress diff output)),
OPT_BOOLEAN(0, source, source, N_(show source)),
+ OPT_BOOLEAN(0, use-mailmap, mailmap, N_(Use mail map
file)),
{ OPTION_CALLBACK, 0, decorate, NULL, NULL, N_(decorate
options),
PARSE_OPT_OPTARG, decorate_callback},
OPT_END()
@@ -136,6 +138,11 @@ static void cmd_log_init_finish(int argc, const char
**argv, const char *prefix,
if (source)
rev-show_source = 1;
+ if (mailmap) {
+ rev-mailmap = xcalloc(1, sizeof(struct string_list));
+ read_mailmap(rev-mailmap, NULL);
+ }
+
if (rev-pretty_given rev-commit_format == CMIT_FMT_RAW) {
/*
* log --pretty=raw is special; ignore UI oriented
--
1.7.12.4.3.g2036a08.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] merge: Honor prepare-commit-msg return code
prepare-commit-msg hook is run when committing to prepare the log
message. If the exit-status is non-zero, the commit should be aborted.
While the script is run before committing a successful merge, the
exit-status is ignored and a non-zero exit doesn't abort the commit.
Abort the commit if prepare-commit-msg returns with a non-zero status
when committing a successful merge.
Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
builtin/merge.c| 5 +++--
t/t7505-prepare-commit-msg-hook.sh | 13 +
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index a96e8ea..3a31c4b 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -800,8 +800,9 @@ static void prepare_to_commit(struct commit_list
*remoteheads)
if (0 option_edit)
strbuf_add_lines(msg, # , comment, strlen(comment));
write_merge_msg(msg);
- run_hook(get_index_file(), prepare-commit-msg,
-git_path(MERGE_MSG), merge, NULL, NULL);
+ if (run_hook(get_index_file(), prepare-commit-msg,
+git_path(MERGE_MSG), merge, NULL, NULL))
+ abort_commit(remoteheads, NULL);
if (0 option_edit) {
if (launch_editor(git_path(MERGE_MSG), NULL, NULL))
abort_commit(remoteheads, NULL);
diff --git a/t/t7505-prepare-commit-msg-hook.sh
b/t/t7505-prepare-commit-msg-hook.sh
index 5b4b694..bc497bc 100755
--- a/t/t7505-prepare-commit-msg-hook.sh
+++ b/t/t7505-prepare-commit-msg-hook.sh
@@ -167,5 +167,18 @@ test_expect_success 'with failing hook (--no-verify)' '
'
+test_expect_success 'with failing hook (merge)' '
+
+ git checkout -B other HEAD@{1}
+ echo more file
+ git add file
+ chmod -x $HOOK
+ git commit -m other
+ chmod +x $HOOK
+ git checkout -
+ head=`git rev-parse HEAD`
+ test_must_fail git merge other
+
+'
test_done
--
1.8.1.rc3.27.g3b73c7d.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
Re: [PATCH] merge: Honor prepare-commit-msg return code
prepare-commit-msg hook is run when committing to prepare the log
message. If the exit-status is non-zero, the commit should be aborted.
I was scratching my head why you CC'ed Jay, until I dug up 65969d4
(merge: honor prepare-commit-msg hook, 2011-02-14).
I did as suggested in SubmittingPatches :)
+test_expect_success 'with failing hook (merge)' '
+
+ git checkout -B other HEAD@{1}
+ echo more file
+ git add file
+ chmod -x $HOOK
I have a feeling that this will break folks without POSIXPERM
prerequisite.
It felt wrong when I did it, but kept it consistent within the file.
It indeed looks older than other test files I've seen so far but I
don't feel like I know the test general-style-and-policy enough to fix
it myself either.
diff --git a/t/t7505-prepare-commit-msg-hook.sh
b/t/t7505-prepare-commit-msg-hook.sh
index bc497bc..3573751 100755
--- a/t/t7505-prepare-commit-msg-hook.sh
+++ b/t/t7505-prepare-commit-msg-hook.sh
@@ -172,11 +172,12 @@ test_expect_success 'with failing hook (merge)' '
git checkout -B other HEAD@{1}
echo more file
git add file
- chmod -x $HOOK
+ rm -f $HOOK
git commit -m other
- chmod +x $HOOK
+ write_script $HOOK -EOF
+ exit 1
+ EOF
git checkout -
- head=`git rev-parse HEAD`
test_must_fail git merge other
'
What about moving the hook file then ? Not very important to me, just
a suggestion as it would keep the shebang.
Cheers,
--
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] Replace git-cvsimport with a rewrite that fixes major bugs.
Doesn't Python come with a standard subprocess module that lets you spawn external programs safely, similar to the way Perl's list form open(), e.g. open($fh, -|, 'git', @args), works? You mean something like this: p1 = subprocess.Popen([backend.command()], stdout=subprocess.PIPE) subprocess.Popen([git, fast-import, --quiet] + gitopts, cwd=outdir, stdin=p1.stdout) Assuming gitopts is a list rather than a string. (care must be taken with backend.command() also) -- 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 (Dec 2012, #08; Mon, 31)
* ap/status-ignored-in-ignored-directory (2012-12-26) 1 commit - wt-status: Show ignored files in untracked dirs A topic still in flux; will be redone. I've already redone this part sending two patches (one with the fix, and one with some tests for each individual use-case) that you probably missed. Here are the message ids: - 1356878341-12942-1-git-send-email-apeli...@gmail.com - 1356878341-12942-2-git-send-email-apeli...@gmail.com * ap/log-mailmap (2012-12-27) 10 commits - log --use-mailmap: optimize for cases without --author/--committer search - log: add log.mailmap configuration option - log: grep author/committer using mailmap - test: Add test for --use-mailmap option - log: Add --use-mailmap option - pretty: Use mailmap to display username and email - mailmap: Add mailmap structure to rev_info and pp - mailmap: Simplify map_user() interface - mailmap: Remove buffer length limit in map_user - Use split_ident_line to parse author and committer (this branch is used by jc/mailmap.) Clean up various codepaths around mailmap and teach the log machinery to use it. Will merge to 'next'. I'm not sure that should be merged to next yet. I've thought of another optimization that will require another preparatory step. Here is the idea: - Create some string_list_lookup_extended with a n parameter (size of the string to match) and a case parameter (to allow strncasecmp() the strings). - Re-re-factor map_user() to take/return pointers instead of strbufs to avoid a bunch of copies. (that is pointless without the former point). The whole idea would be to avoid a bunch of copies: one for lowering the email, the other for adding '\0' at the end of name before running string_list_lookup(). Cheers, -- 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] dir.c: Make git-status --ignored more consistent
The current behavior of git-status is inconsistent and
misleading. Especially when used with --untracked-files=all option:
- files ignored in untracked directories will be missing from status
output.
- untracked files in committed yet ignored directories are also
missing.
- with --untracked-files=normal, untracked directories that contains
only ignored files are dropped too.
Make the behavior more consistent across all possible use cases:
- --ignored --untracked-files=normal doesn't show each specific
files but top directory.
Shows untracked directories that only contains ignored files, and
ignored tracked directories with untracked files.
- --ignored --untracked-files=all shows all ignored files, either
because it's in an ignored directory (tracked or untracked), or
because the file is explicitly ignored.
Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
dir.c | 98 +++
wt-status.c |4 ++-
2 files changed, 81 insertions(+), 21 deletions(-)
diff --git a/dir.c b/dir.c
index 5a83aa7..d0c92dc 100644
--- a/dir.c
+++ b/dir.c
@@ -834,8 +834,9 @@ static enum exist_status directory_exists_in_index(const
char *dirname, int len)
* traversal routine.
*
* Case 1: If we *already* have entries in the index under that
- * directory name, we always recurse into the directory to see
- * all the files.
+ * directory name, we recurse into the directory to see all the files,
+ * unless the directory is excluded and we want to show ignored
+ * directories
*
* Case 2: If we *already* have that directory name as a gitlink,
* we always continue to see it as a gitlink, regardless of whether
@@ -849,6 +850,9 @@ static enum exist_status directory_exists_in_index(const
char *dirname, int len)
* just a directory, unless hide_empty_directories is
* also true and the directory is empty, in which case
* we just ignore it entirely.
+ * if we are looking for ignored directories, look if it
+ * contains only ignored files to decide if it must be shown as
+ * ignored or not.
* (b) if it looks like a git directory, and we don't have
* 'no_gitlinks' set we treat it as a gitlink, and show it
* as a directory.
@@ -861,12 +865,15 @@ enum directory_treatment {
};
static enum directory_treatment treat_directory(struct dir_struct *dir,
- const char *dirname, int len,
+ const char *dirname, int len, int exclude,
const struct path_simplify *simplify)
{
/* The len-1 is to strip the final '/' */
switch (directory_exists_in_index(dirname, len-1)) {
case index_directory:
+ if ((dir-flags DIR_SHOW_OTHER_DIRECTORIES) exclude)
+ break;
+
return recurse_into_directory;
case index_gitdir:
@@ -886,7 +893,23 @@ static enum directory_treatment treat_directory(struct
dir_struct *dir,
}
/* This is the show_other_directories case */
- if (!(dir-flags DIR_HIDE_EMPTY_DIRECTORIES))
+
+ /*
+* We are looking for ignored files and our directory is not ignored,
+* check if it contains only ignored files
+*/
+ if ((dir-flags DIR_SHOW_IGNORED) !exclude) {
+ int ignored;
+ dir-flags = ~DIR_SHOW_IGNORED;
+ dir-flags |= DIR_HIDE_EMPTY_DIRECTORIES;
+ ignored = read_directory_recursive(dir, dirname, len, 1,
simplify);
+ dir-flags = ~DIR_HIDE_EMPTY_DIRECTORIES;
+ dir-flags |= DIR_SHOW_IGNORED;
+
+ return ignored ? ignore_directory : show_directory;
+ }
+ if (!(dir-flags DIR_SHOW_IGNORED)
+ !(dir-flags DIR_HIDE_EMPTY_DIRECTORIES))
return show_directory;
if (!read_directory_recursive(dir, dirname, len, 1, simplify))
return ignore_directory;
@@ -894,6 +917,49 @@ static enum directory_treatment treat_directory(struct
dir_struct *dir,
}
/*
+ * Decide what to do when we find a file while traversing the
+ * filesystem. Mostly two cases:
+ *
+ * 1. We are looking for ignored files
+ * (a) File is ignored, include it
+ * (b) File is in ignored path, include it
+ * (c) File is not ignored, exclude it
+ *
+ * 2. Other scenarios, include the file if not excluded
+ *
+ * Return 1 for exclude, 0 for include.
+ */
+static int treat_file(struct dir_struct *dir, struct strbuf *path, int
exclude, int *dtype)
+{
+ struct path_exclude_check check;
+ int exclude_file = 0;
+
+ if (exclude)
+ exclude_file = !(dir-flags DIR_SHOW_IGNORED);
+ else if (dir-flags DIR_SHOW_IGNORED) {
+ /*
+* Optimization:
+* Don't spend time on indexed files, they won't be
+* added to the list anyway
+*/
+ struct cache_entry *ce = index_name_exists(the_index,
+ path-buf
