Re: [PATCH v2 17/21] Convert refresh_index to take struct pathspec
On Fri, Jan 11, 2013 at 06:21:11PM +0700, Nguyễn Thái Ngọc Duy wrote:
- for (i = 0; i specs; i++) {
+ for (i = 0; i pathspec-nr; i++) {
if (!seen[i])
- die(_(pathspec '%s' did not match any files),
pathspec[i]);
+ die(_(pathspec '%s' did not match any files),
pathspec-raw[i]);
}
This needs the following fixup on top. I don't want to send another
reroll just a couple hours after I flooded git@vger. I did not plan to
work on the series this soon but somehow another problem got me back
here.
-- 8 --
diff --git a/builtin/add.c b/builtin/add.c
index 1235eb9..e1bcdb9 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -159,7 +159,8 @@ static void refresh(int verbose, const struct pathspec
*pathspec)
pathspec, seen, _(Unstaged changes after refreshing the
index:));
for (i = 0; i pathspec-nr; i++) {
if (!seen[i])
- die(_(pathspec '%s' did not match any files),
pathspec-raw[i]);
+ die(_(pathspec '%s' did not match any files),
+ pathspec-items[i].match);
}
free(seen);
}
-- 8 --
and the baaad reason: pathspec-items[] are sorted because of 86e4ca6
(tree_entry_interesting(): fix depth limit with overlapping pathspecs
- 2010-12-15). But raw[] are _not_. So raw[i] does not correspond to
item[i].
Now seen[] array returned from match_pathspec() has the order
corresponding to raw[]. On the other hand match_pathspec_depth()
returns seen[] corresponds to items[]. This patch converts
match_pathspec() to match_pathspec_depth() so we need to use the
correct pathspec array.
I'll put these explanation in the next reroll. And don't worry about
this subtle difference. My next email kills match_pathspec() for good.
--
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] pack-objects: compression level for non-blobs
On Sun, Dec 30, 2012 at 7:05 PM, Jeff King p...@peff.net wrote: So I was thinking about this, which led to some coding, which led to some benchmarking. I like your way of thinking! May I suggest you take a new year break first, then think about reachability bitmaps ;-) 2013 will be an exciting year. I want to clean up a few things in the code before I post it, but the general idea is to have arbitrary per-pack cache files in the objects/pack directory. Like this: $ cd objects/pack ls pack-a3e262f40d95fc0cc97d92797ff9988551367b75.commits pack-a3e262f40d95fc0cc97d92797ff9988551367b75.idx pack-a3e262f40d95fc0cc97d92797ff9988551367b75.pack pack-a3e262f40d95fc0cc97d92797ff9988551367b75.parents pack-a3e262f40d95fc0cc97d92797ff9988551367b75.timestamps pack-a3e262f40d95fc0cc97d92797ff9988551367b75.trees Each file describes the objects in the matching pack. If a new pack is generated, you'd throw away the old cache files along with the old pack, and generate new ones. Or not. These are totally optional, and an older version of git will just ignore them. A newer version will use them if they're available, and otherwise fallback to the existing code (i.e., reading the whole object from the pack). So you can generate them at You have probably thought about this (and I don't have the source to check first), but we may need to version these extra files so we can change the format later if needed. Git versions that do not recognize new versions simply ignore the cahce. repack time, later on, or not at all. For now I have a separate command that generates them based on the pack index; if this turns out to be a good idea, it would probably get called as part of repack. I'd like to make it part of index-pack, where we have nearly everything in memory. But let's leave it as a separate command first. Each file is a set of fixed-length records. The commits file contains the sha1 of every commit in the pack (sorted). A binary search of the mmap'd file gives the position of a particular commit within the list, I think we could avoid storing sha-1 in the cache with Shawn's idea [1]. But now I read it again I fail to see it :( [1] http://article.gmane.org/gmane.comp.version-control.git/206485 Of course, it does very little for the full --objects listing, where we spend most of our time inflating trees. We could couple this with uncompressed trees (which are not all that much bigger, since the sha1s do not compress anyway). Or we could have an external tree cache, but I'm not sure exactly what it would look like (this is basically reinventing bits of packv4, but doing so in a way that is redundant with the existing packfile, rather than replacing it). Depending on the use case, we could just generate packv4-like cache for recently-used trees only. I'm not sure how tree cache impact a merge operation on a very large worktree (iow, a lot of trees referenced from HEAD to be inflated). This is something a cache can do, but a new pack version cannot. Or since the point of --objects is usually reachability, it may make more sense to pursue the bitmap, which should be even faster still. Yes. And if narrow clone ever comes, which needs --objects limited by pathspec, we could just produce extra bitmaps for frequently-used pathspecs and only allow narrow clone with those pathspecs. -- Duy -- 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] pack-objects: compression level for non-blobs
On Sat, Dec 29, 2012 at 7:41 AM, Jeff King p...@peff.net wrote: I wonder if we could do even better, though. For a traversal, we only need to look at the commit header. We could potentially do a progressive inflate and stop before getting to the commit message (which is the bulk of the data, and the part that is most likely to benefit from compression). Commit cache should solve this efficiently as it also eliminates parsing cost. We discussed this last time as a side topic of the reachability bitmap feature. -- Duy -- 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] pack-objects: compression level for non-blobs
On Sat, Dec 29, 2012 at 12:07 PM, Jeff King p...@peff.net wrote: On Sat, Dec 29, 2012 at 11:34:09AM +0700, Nguyen Thai Ngoc Duy wrote: On Sat, Dec 29, 2012 at 7:41 AM, Jeff King p...@peff.net wrote: I wonder if we could do even better, though. For a traversal, we only need to look at the commit header. We could potentially do a progressive inflate and stop before getting to the commit message (which is the bulk of the data, and the part that is most likely to benefit from compression). Commit cache should solve this efficiently as it also eliminates parsing cost. We discussed this last time as a side topic of the reachability bitmap feature. I agree that a commit cache would solve this (though it can not help the tree traversal). Yeah, caching trees efficiently is not easy. But just dropping the compression (or doing partial decompression when we only care about the beginning part) is way less code and complexity. I think I tried the partial decompression for commit header and it did not help much (or I misremember it, not so sure). There's no cache to manage. If reachability bitmap is implemented, we'll have per-pack cache infrastructure ready, so less work there for commit cache. -- Duy -- 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 8/8] wildmatch: advance faster in asterisk + literal patterns
On Fri, Dec 28, 2012 at 1:24 PM, Junio C Hamano gits...@pobox.com wrote:
+ while ((t_ch = *text) != '\0'
+(!(flags WM_PATHNAME) || t_ch
!= '/')) {
Why do we look at (flags WM_PATHMAME) and not special here?
Because I was careless. Thanks for spotting it. I'll fix it and add
some more tests about **literal with WM_PATHNAME.
--
Duy
--
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 4/8] wildmatch: support no FNM_PATHNAME mode
On Fri, Dec 28, 2012 at 1:24 PM, Junio C Hamano gits...@pobox.com wrote:
if (*++p == '*') {
const uchar *prev_p = p - 2;
while (*++p == '*') {}
- if ((prev_p == text || *prev_p == '/') ||
+ if (!(flags WM_PATHNAME))
+ /* without WM_PATHNAME, '*' == '**' */
+ special = 1;
+ else if ((prev_p == text || *prev_p == '/') ||
Not a new issue in this patch,
No, it's an issue from nd/wildmatch, 40bbee0 (wildmatch: adjust **
behavior - 2012-10-15).
but here, prev_p points into the
pattern string, two bytes before p, which is the byte before the
** that we are looking at (which might be before the beginning of
the pattern). text is the string we are trying to match that
pattern against. How can these two pointers be compared to yield a
meaningful value?
They can't. I wanted to check whether ** is at the start of the
pattern (so no preceding '/' needed) and used a wrong pointer to
compare to. Funny there is a test about this and it does not catch it
because prev_p access something before the pattern. Will fix.
(*p == '\0' || *p == '/' ||
(p[0] == '\\' p[1] == '/'))) {
OK. **/, ** (end of pattern), and **\/ are handled here.
Do we have to worry about **[/] the same way, or a class never
matches the directory separator, even if it is a singleton class
that consists of '/' (which is fine by me)? If so, is \/ more or
less like [/]?
This is a special case of ** with FNM_PATHNAME on. With
FNM_PATHNAME, '[]' and '?' cannot match '/' so any patterns with '[/]'
match nothing. I think we don't need to worry about this case.
--
Duy
--
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: Find the starting point of a local branch
On Mon, Dec 24, 2012 at 1:19 PM, Jeff King p...@peff.net wrote:
On Mon, Dec 24, 2012 at 12:28:45PM +0700, Nguyen Thai Ngoc Duy wrote:
You want to know what commit was I at when I typed `git branch
mybranch`? The problem is git doesn't record this information and
doesn't have the slightest clue.
Maybe we should store this information. reflog is a perfect place for
this, I think. If this information is reliably available, git rebase
can be told to rebase my whole branch instead of my choosing the
base commit for it.
Is that what you really want, though? We record the upstream branch
already, and you can calculate the merge base with that branch to see
which commits are unique to your branch. In simple cases, that is the
same as where did I start the branch. In more complex cases, it may
not be (e.g., if you merged some of the early commits in the branch
already). But in that latter case, would you really want to rebase
those commits that had been merged?
The reason that git does not bother storing where did I start this
branch is that it is usually not useful. The right question is usually
what is the merge base. There are exceptions, of course (e.g., if you
are asking something like what work did I do while checked out on the
'foo' branch). But for merging and rebasing, I think the computed
merge-base is much more likely to do what people want.
Rebasing is exactly why I want this. Merge base works most of the time
until you rewrite upstream (which I do sometimes). There are also
cases when I create a branch without upstream, or when upstream is
renamed. Still, making rebase -i --topic == rebase -i $(git
merge-base HEAD @{upstream}) would be cool.
--
Duy
--
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: Find the starting point of a local branch
On Mon, Dec 24, 2012 at 1:27 PM, Junio C Hamano gits...@pobox.com wrote:
Nguyen Thai Ngoc Duy pclo...@gmail.com writes:
On Mon, Dec 24, 2012 at 12:34 PM, Tomas Carnecky
tomas.carne...@gmail.com wrote:
Maybe we should store this information. reflog is a perfect place for
this, I think. If this information is reliably available, git rebase
can be told to rebase my whole branch instead of my choosing the
base commit for it.
What's the starting point of the branch if I type: git branch foo
commit-ish?
You start working off commit-ish so I think the starting point would
be commit-ish.
Yeah, that sounds sensible. Don't we already have it in the reflog,
though?
I looked briefly at reflog before writing my previous mail and noticed
that when I create a new branch (usually using git checkout -b branch
ref) it does not record the base commit.
What is trickier is when you later transplant it to some other base
(perhaps prepare a topic on 'master', realize it should better apply
to 'maint' and move it there). If the user did the transplanting by
hand, reflog would probably not have enough information, e.g. after
$ git checkout maint^0
$ git log --oneline master..topic
$ git cherry-pick $one_of_the_commit_names_you_see_in_the_above
$ git cherry-pick $another_commit_name_you_see_in_the_above
...
$ git branch -f topic
no reflog other than HEAD@{} will tell you that you were at maint^0,
so the reflog of topic wouldn't know it forked from there, either.
We could at least invalidate the recorded base in reflog and let user
define a new one (I hope).
--
Duy
--
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: Find the starting point of a local branch
On Tue, Dec 25, 2012 at 2:10 AM, Junio C Hamano gits...@pobox.com wrote:
I looked briefly at reflog before writing my previous mail and noticed
that when I create a new branch (usually using git checkout -b branch
ref) it does not record the base commit.
Hmph. Perhaps you are referring to something different than what I
think the base commit with that word.
$ git reflog mz/pick-unborn | tail -n 1
b3cf6f3 mz/pick-unborn@{3}: branch: Created from ko/master
No you're right. My reflogs must be pruned. Creating a new branch does
produce that entry.
We could at least invalidate the recorded base in reflog and let user
define a new one (I hope).
Please do not even think about going back and rewrite to lose
information. If the records have full information, you should be
able to reconstruct what you want from it without rewriting.
Even more importantly, wish to invalidate indicates that you know
at a newer point that you have more authoritative information than
the older reflog entries, so you should be able to do the moral
equivalent by writing the event as establishing a new base at that
point (e.g. checkout -B), and stopping at that point in the reflog
when reading, without losing the older reflog entries.
Exactly. And when we prune the reflog, we could add the base back so
the base is always in reflog.
--
Duy
--
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: Find the starting point of a local branch
On Mon, Dec 24, 2012 at 11:09 AM, Seth Robertson in-gitv...@baka.org wrote: In message 20121224035825.GA17203@zuhnb712, Woody Wu writes: How can I find out what's the staring reference point (a commit number or tag name) of a locally created branch? I can use gitk to find out it but this method is slow, I think there might be a command line to do it quickly. The answer is more complex than you probably suspected. Technically, `git log --oneline mybranch | tail -n 1` will tell you the starting point of any branch. But...I'm sure that isn't what you want to know. You want to know what commit was I at when I typed `git branch mybranch`? The problem is git doesn't record this information and doesn't have the slightest clue. Maybe we should store this information. reflog is a perfect place for this, I think. If this information is reliably available, git rebase can be told to rebase my whole branch instead of my choosing the base commit for it. -- Duy -- 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: Find the starting point of a local branch
On Mon, Dec 24, 2012 at 12:34 PM, Tomas Carnecky tomas.carne...@gmail.com wrote: Maybe we should store this information. reflog is a perfect place for this, I think. If this information is reliably available, git rebase can be told to rebase my whole branch instead of my choosing the base commit for it. What's the starting point of the branch if I type: git branch foo commit-ish? You start working off commit-ish so I think the starting point would be commit-ish. -- Duy -- 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 3/3] Convert all fnmatch() calls to wildmatch()
On Thu, Dec 20, 2012 at 1:36 AM, Junio C Hamano gits...@pobox.com wrote:
@@ -627,7 +628,7 @@ enum interesting tree_entry_interesting(const struct
name_entry *entry,
return entry_interesting;
if (item-use_wildcard) {
- if (!fnmatch(match + baselen, entry-path, 0))
+ if (!wildmatch(match + baselen, entry-path,
0))
return entry_interesting;
This and the change to dir.c obviously have interactions with
8c6abbc (pathspec: apply *.c optimization from exclude,
2012-11-24).
I've already alluded to it in my response to 2/3, I guess.
Conflict of plans. I did not expect myself to work on replacing
fnmatch soon and git_fnmatch is an intermediate step to collect more
optimizations and gradually replace fnmatch. Eventually git_fnmatch()
would become a wrapper of wildmatch, all the optimizations are pushed
down there. If we replace fnmatch-wildmatch first then there's little
reason for the existence of git_fnmatch(). Maybe I should merge this
with the fnmatch elimination into a single series.
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] wildmatch: support no FNM_PATHNAME mode
On Thu, Dec 20, 2012 at 8:55 AM, Nguyen Thai Ngoc Duy pclo...@gmail.com wrote: As long as simpler patterns fnmatch() groks (namely, '?', '*', and '[class]' wildcards only) are not slowed down by replacing it with wildmatch(), that is, of course. I'm concerned about performance vs fnmatch too. I'll probably write a small program to exercise both and measure it with glibc. I'll send out proper patches later this weekend, but unless I make big mistakes it seems wildmatch is faster than fnmatch from glibc 2.14.1 on x86-64 :) Maybe I did make mistakes. The test matches every line in /tmp/filelist.txt (which is ls-files from linux-2.6.git) 2000 times. pathname means FNM_PATHNAME. $ export LANG=C $ ./test-wildmatch-perf /tmp/filelist.txt 'Documentation/*' 2000 wildmatch 1s 528394us fnmatch 2s 588396us $ ./test-wildmatch-perf /tmp/filelist.txt 'drivers/*' 2000 wildmatch 1s 957243us fnmatch 3s 134839us $ ./test-wildmatch-perf /tmp/filelist.txt 'Documentation/*' 2000 pathname wildmatch 1s 548575us fnmatch 2s 629680us $ ./test-wildmatch-perf /tmp/filelist.txt 'drivers/*' 2000 pathname wildmatch 2s 142377us fnmatch 3s 318295us $ ./test-wildmatch-perf /tmp/filelist.txt '[Dd]ocu[Mn]entation/*' 2000 wildmatch 1s 748505us fnmatch 3s 224414us $ ./test-wildmatch-perf /tmp/filelist.txt '[Dd]o?u[Mn]en?ati?n/*' 2000 wildmatch 1s 743268us fnmatch 3s 278034us $ ./test-wildmatch-perf /tmp/filelist.txt '[Dd]o?u[Mn]en?ati?n/*' 2000 pathname wildmatch 1s 745151us fnmatch 3s 314127us $ ./test-wildmatch-perf /tmp/filelist.txt nonsense 2000 pathname wildmatch 1s 310727us fnmatch 2s 279123us -- Duy -- 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/WIP 0/3] Bye bye fnmatch()
On Wed, Dec 19, 2012 at 8:08 PM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote: For those who have not followed, nd/wildmatch brings another fnmatch-like implementation which can nearly replace fnmatch. System fnmatch() seems to behave differently in some cases. It's better to stay away and use one implementation for all. Oh I forgot. Case-insensitive matching will be available to everybody. On the other hand it'll be a lot more work to (implement and) use other FNM_* flags like FNM_PERIOD. -- Duy -- 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] add core.pathspecGlob config option
On Thu, Dec 20, 2012 at 3:34 AM, Jeff King p...@peff.net wrote: Part of me thinks this is just gross, because :(noglob) is the right solution. But after spending a few hours trying it this morning, there is a ton of refactoring required to make it work correctly everywhere (although we could die() if we see it in places that are not using init_pathspec, so at least we could say not supported yet and not just silently ignore it). Yep, I'm still half way to converting everything to the new pathspec code. I'm not there yet. And things like this probably better goes with a config key or command line option, appending :(noglob) to every pathspec item is not pleasant. :(icase) may go the same way. So I think this is a nice, simple approach for sites that want it, and noglob magic can come later (and will not be any harder to implement as a result of this patch). Any chance to make use of nd/pathspec-wildcard? It changes the same code path in match_one. If you base on top of nd/pathspec-wildcard, all you have to do is assign nowildcard_len to len (i.e. no wildcard part). -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] wildmatch: support no FNM_PATHNAME mode
On Thu, Dec 20, 2012 at 12:24 AM, Junio C Hamano gits...@pobox.com wrote: When that happens, we should want to retain the same do not bother to descend into subdirectories that will never match optimization for a pattern like Doc*tion/**/*.txt. Because of FNM_PATHNAME, we can tell if a subdirectory is worth descending into by looking at the not-so-simple prefix Doc*tion/; Documentation will match, Doc will not (because '*' won't match '/'). Which tells me that integrating this _well_ into the rest of the system is not just a matter of replacing fnmatch() with wildmatch(). Yeah, we open a door for more opportunities and a lot more headache. I also expect that wildmatch() would be much slower than fnmatch() especially when doing its ** magic, but I personally do not think it will be a showstopper. A potential showstopper is the lack of multibyte support. I don't know if people want that though. If the user asks for a more powerful but expensive operation, we are saving time for the user by doing a more powerful thing (reducing the need to postprocess the results) and can afford to spend extra cycles. In some case we may be able to spend fewer cycles by preprocessing patterns first. As long as simpler patterns fnmatch() groks (namely, '?', '*', and '[class]' wildcards only) are not slowed down by replacing it with wildmatch(), that is, of course. I'm concerned about performance vs fnmatch too. I'll probably write a small program to exercise both and measure it with glibc. -- Duy -- 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] log --format: teach %C(auto,black) to paint it black only on terminals
On Mon, Dec 17, 2012 at 3:40 PM, Junio C Hamano gits...@pobox.com wrote:
Traditionally, %C(color attr) always emitted the ANSI color
sequence; it was up to the scripts that wanted to conditionally
color their output to omit %C(...) specifier when they do not want
colors.
Optionally allow auto, to be prefixed to the color, so that the
output is colored iff it goes to the terminal.
I see prefix is clearly documented. Still it feels a bit unnatural
that %C(red,auto) won't work. But we can make that case work later if
somebody cares enough.
if (!end)
return 0;
- color_parse_mem(placeholder + 2,
- end - (placeholder + 2),
+ if (!memcmp(begin, auto,, 5)) {
+ if (!want_color(-1))
+ return end - placeholder + 1;
This want_color() checks color.ui and only when color.ui = auto, it
bothers to check if the output is tty. I think the document should say
that auto, (or maybe another name because it's not really auto)
respects color.ui.
--
Duy
--
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 and History Simplification
On Mon, Dec 17, 2012 at 6:33 PM, Dinesh Subramani dinesh.subram...@gmail.com wrote: I am using the below command : git log --stat --decorate=full --since=date Can you please let me know if the above command will list all the commits and would not skip any of the commits due to History Simplification. Any help would be very useful. If my memory is still functioning, history simplification only takes place when you specify pathspec. The above command does not have pathspec, so no history simplification. -- Duy -- 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 3/4] cache-tree: fix writing cache-tree when CE_REMOVE is present
On Sun, Dec 16, 2012 at 2:20 PM, Junio C Hamano gits...@pobox.com wrote:
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
entry_count is used in update_one() for two purposes:
1. to skip through the number of processed entries in in-memory index
2. to record the number of entries this cache-tree covers on disk
Unfortunately when CE_REMOVE is present these numbers are not the same
because CE_REMOVE entries are automatically removed before writing to
disk but entry_count is not adjusted and still counts CE_REMOVE
entries.
Nicely explained. I wonder if we can also add a piece of test to
the patch 4/4 to demonstrate the issue with CE_REMOVE entries,
though.
A hand crafted one, maybe. I did not attempt to recreate it with git
commands (and I don't think we update cache-tree after unpack_trees).
So I wrote something like this instead:
int main(int ac, char **av)
{
unsigned char sha1[20];
setup_git_directory();
read_cache();
active_cache[1]-ce_flags |= CE_REMOVE;
write_cache_as_tree(sha1, 0, NULL);
return 0;
}
I can polish it a bit and write new tests based on it and
test-dump-cache-tree if you want.
--
Duy
--
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, #03; Wed, 12)
On Sat, Dec 15, 2012 at 2:45 PM, Felipe Contreras felipe.contre...@gmail.com wrote: Couple of facts first: a) This code was already merged b) This code is for a test c) I'm the only developer so far Cruft in the codebase is a problem for git *developers* because it makes the code harder to maintain and extend. A problem big enough to warrant the rejection of the patch series? No. May I suggest you maintain remote-bzr as a separate project? You have total control that way without anyone's disagreeing with you. So you may be more productive and we have less of these emails back and forth. And speaking from someone whose series may take months to get in, why the rush? -- Duy -- 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] t3070: Disable some failing fnmatch tests
On Sun, Dec 16, 2012 at 2:19 AM, Ramsay Jones ram...@ramsay1.demon.co.uk wrote: The failing tests make use of a POSIX character class, '[:xdigit:]' in this case, which some versions of the fnmatch() library function do not support. In the spirit of commit f1cf7b79 (t3070: disable unreliable fnmatch tests, 15-10-2012), we disable the fnmatch() half of these tests. I have no problem with this. You're on cygwin, right? -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/2] cache-tree: invalidate i-t-a paths after generating trees
On Thu, Dec 13, 2012 at 9:04 AM, Junio C Hamano gits...@pobox.com wrote:
@@ -324,7 +325,14 @@ static int update_one(struct cache_tree *it,
if (!sub)
die(cache-tree.c: '%.*s' in '%s' not found,
entlen, path + baselen, path);
- i += sub-cache_tree-entry_count - 1;
+ i--; /* this entry is already counted in sub */
Huh?
The -1 in the original is the bog-standard compensation for the
for(;;i++) loop.
Exactly. It took me a while to figure out what - 1 was for and I
wanted to avoid that for other developers. Only I worded it badly.
I'll replace the for loop with a while loop to make it clearer...
+ if (sub-cache_tree-entry_count 0) {
+ i -= sub-cache_tree-entry_count;
+ sub-cache_tree-entry_count = -1;
+ to_invalidate = 1;
+ }
+ else
+ i += sub-cache_tree-entry_count;
While the rewritten version is not *wrong* per-se, I have a feeling
that it may be much easier to read if written like this:
if (sub-cache_tree_entry_count 0) {
to_invalidate = 1;
to_skip = 0 - sub-cache_tree_entry_count;
sub-cache_tree_entry_count = -1;
} else {
to_skip = sub-cache_tree_entry_count;
}
i += to_skip - 1;
..or this would be fine too. Which way to go?
A while we're still at the cache tree
- if (ce-ce_flags (CE_REMOVE | CE_INTENT_TO_ADD))
- continue; /* entry being removed or placeholder */
+ /*
+* CE_REMOVE entries are removed before the index is
+* written to disk. Skip them to remain consistent
+* with the future on-disk index.
+*/
+ if (ce-ce_flags CE_REMOVE)
+ continue;
+
A CE_REMOVE entry is removed later from the index, however it is still
counted in entry_count. entry_count serves two purposes: to skip the
number of processed entries in the in-memory index, and to record the
number of entries in the on-disk index. These numbers do not match
when CE_REMOVE is present. We have correct in-memory entry_count,
which means incorrect on-disk entry_count in this case.
I tested with an index that has a/b and a/c. The latter has CE_REMOVE.
After writing cache tree I get:
$ git ls-files --stage
100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 a/b
$ ../test-dump-cache-tree
878e27c626266ac04087a203e4bdd396dcf74763 (2 entries, 1 subtrees)
878e27c626266ac04087a203e4bdd396dcf74763 #(ref) (1 entries, 1 subtrees)
4277b6e69d25e5efa77c455340557b384a4c018a a/ (2 entries, 0 subtrees)
4277b6e69d25e5efa77c455340557b384a4c018a #(ref) a/ (1 entries, 0 subtrees)
If I throw out that index, create a new one with a/b alone and write-tree, I get
$ ../test-dump-cache-tree
878e27c626266ac04087a203e4bdd396dcf74763 (1 entries, 1 subtrees)
4277b6e69d25e5efa77c455340557b384a4c018a a/ (1 entries, 0 subtrees)
Shall we fix this too? I'm thinking of adding skip argument to update_one and
i += sub-cache_tree-entry_count - 1;
would become
i += sub-cache_tree-entry_count + skip - 1;
and entry_count would always reflect on-disk value. This skip can be
reused for this i-t-a patch as well.
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 0/3] compiling git with gcc -O3 -Wuninitialized
On Sat, Dec 15, 2012 at 5:09 AM, Jeff King p...@peff.net wrote:
I always compile git with gcc -Wall -Werror, because it catches a lot
of dubious constructs, and we usually keep the code warning-free.
However, I also typically compile with -O0 because I end up debugging
a fair bit.
Sometimes, though, I compile with -O3, which yields a bunch of new
variable might be used uninitialized warnings. What's happening is
that as functions get inlined, the compiler can do more static analysis
of the variables. So given two functions like:
int get_foo(int *foo)
{
if (something_that_might_fail() 0)
return error(unable to get foo);
*foo = 0;
return 0;
}
void some_fun(void)
{
int foo;
if (get_foo(foo) 0)
return -1;
printf(foo is %d\n, foo);
}
If get_foo() is not inlined, then when compiling some_fun, gcc sees only
that a pointer to the local variable is passed, and must assume that it
is an out parameter that is initialized after get_foo returns.
However, when get_foo() is inlined, the compiler may look at all of the
code together and see that some code paths in get_foo() do not
initialize the variable. And we get the extra warnings.
Other options:
- Any __attribute__ or #pragma to aid flow analysis (or would gcc dev be
willing to add one)?
- Maintain a list of false positives and filter them out from gcc output?
And if we do this, should we support other compilers as well? I tried
clang once a long while ago and got a bunch of warnings iirc.
--
Duy
--
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: FW: Git log --graph doesn't output color when redirected
On Thu, Dec 13, 2012 at 8:13 PM, Jeff King p...@peff.net wrote: If you are using --format=%C(red) or similar placeholders, they are the odd duck by not respecting the auto-color mode. But they should, shouldn't they? Just asking. I may do it to when I revive nd/pretty-placeholder-with-color-option. -- Duy -- 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: Python extension commands in git - request for policy change
On Wed, Dec 12, 2012 at 7:53 AM, Tomas Carnecky
tomas.carne...@gmail.com wrote:
If it doesn't, it would be trivial to add. It's a one-liner. It's been a while
since I used Lua, but it would be something like this:
void L_putenv(lua_State *L) {
putenv(lua_tostring(L, 1));
}
and then somewhere during setup:
lua_register(L, putenv, L_putenv);
I should have done my homework before asking, but well.. is there any
way to automate this? If we use lua for writing builtin commands,
we'll need to export a lot of C functions and writing wrappers like
this is boring and time consuming. Also, assume I export fn(char*,int)
to Lua, then I change the prototype to fn(char*, char*), can Lua spot
all the call sites at compile time (or something) so I can update
them?
--
Duy
--
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] cache-tree: invalidate i-t-a paths after generating trees
On Mon, Dec 10, 2012 at 1:50 PM, Junio C Hamano gits...@pobox.com wrote:
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
diff --git a/cache-tree.c b/cache-tree.c
index 28ed657..989a7ff 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -248,6 +248,7 @@ static int update_one(struct cache_tree *it,
int missing_ok = flags WRITE_TREE_MISSING_OK;
int dryrun = flags WRITE_TREE_DRY_RUN;
int i;
+ int to_invalidate = 0;
if (0 = it-entry_count has_sha1_file(it-sha1))
return it-entry_count;
@@ -324,7 +325,13 @@ static int update_one(struct cache_tree *it,
if (!sub)
die(cache-tree.c: '%.*s' in '%s' not found,
entlen, path + baselen, path);
- i += sub-cache_tree-entry_count - 1;
+ i--; /* this entry is already counted in sub */
+ if (sub-cache_tree-entry_count 0) {
+ i -= sub-cache_tree-entry_count;
+ to_invalidate = 1;
+ }
+ else
+ i += sub-cache_tree-entry_count;
Hrm. update_one() is prepared to see a cache-tree whose entry count
is zero (see the context lines in the previous hunk) and the
invariant for the rest of the code is if 0 = entry_count, the
cached tree is valid; invalid cache-tree has -1 in entry_count.
More importantly, entry_count negated does not in general express
how many entries there are in the subtree and does not tell us how
many index entries to skip.
Yeah I use entry_count for two different things here. In the previous
(unsent) version of the patch I had entry_count = -1 right after i
-= entry_count
+ if (sub-cache_tree-entry_count 0) {
+ i -= sub-cache_tree-entry_count;
+ sub-cache_tree-entry_count = -1;
+ to_invalidate = 1;
+ }
which makes it clearer that the use of negative entry count is only
valid within update_one. Then I changed my mind because it says
'negative means invalid' in cache-tree.h. So, put entry_count = -1
back or just add another field to struct cache_tree for this?
--
Duy
--
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] git(1): remove a defunct link to list of authors
On Mon, Dec 10, 2012 at 1:31 PM, Junio C Hamano gits...@pobox.com wrote: Nguyen Thai Ngoc Duy pclo...@gmail.com writes: On Sat, Dec 8, 2012 at 12:59 AM, Junio C Hamano gits...@pobox.com wrote: * If somebody has a working replacement URL, we could use that instead, of course. Takers? A possible alternative could be https://www.ohloh.net/p/git/contributors/summary Nice charts! Yup. Their numbers seem to be just 'any commit by the author, with mailmap applied', and I am of two minds with it. Counting without shortlog --no-merges, depending on the management style of the project, tends to credit the integrator too much. Even though vetting the patches and choosing when to merge the topics is a significant contribution, it isn't *that* big compared to the work done by the contributor who took initiative to scratch that itch. With or without --no-merges, the big picture you can get out of git shortlog -s -n --since=1.year does not change very much, but the headline numbers give a wrong impression. These numbers are approximate anyway. Commit counts or the number of changed lines do not accurately reflect the effort in many cases. And about merges, in this particular case of Git where the maintainer imo has done an excellent job as a guard, I'd say it's the credit for reviewing, not simply merging. But not using the link is fine too. We can wait for Jeff's patch to be merged. And of course, application of the mailmap is very important, if you want to get meaningful numbers out of shortlog over a longer period. -- Duy -- 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] git(1): remove a defunct link to list of authors
On Sat, Dec 8, 2012 at 12:59 AM, Junio C Hamano gits...@pobox.com wrote: * If somebody has a working replacement URL, we could use that instead, of course. Takers? A possible alternative could be https://www.ohloh.net/p/git/contributors/summary Nice charts! -- Duy -- 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] Add basic syntax check on shell scripts
On Wed, Dec 5, 2012 at 2:39 AM, Junio C Hamano gits...@pobox.com wrote: Or a project commit hook? Surely. It is OK to have cd t make test-lint in your pre-commit hook. No, what I meant is a shared pre-commit script that all git devs are encouraged (or forced) to install so bugs are found locally rather than after patches are sent to you. The hook content does not really matter. -- Duy -- 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] cache-tree: invalidate i-t-a paths after writing trees
On Fri, Nov 30, 2012 at 7:06 AM, Junio C Hamano gits...@pobox.com wrote: Nguyen Thai Ngoc Duy pclo...@gmail.com writes: An alternative might be to add a phoney bit next to used in the cache_tree structure, mark the cache tree as phoney when we skip an entry marked as CE_REMOVE or CE_ITA, and make the postprocessing loop this patch adds aware of that bit, instead of iterating over the index entries; instead, it would recurse the resulting cache tree and invalidate parts of the tree that have subtrees with the phoney bit set, or something. Yeah, that sounds better. Did anything happen to this topic after this? Not from my side because I forgot to mark this thread as a todo item and unsurprisingly forgot about it. -- Duy -- 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] fsck: warn about '.' and '..' in trees
On Wed, Nov 28, 2012 at 9:27 AM, Jeff King p...@peff.net wrote: A tree with meta-paths like '.' or '..' does not work well with git; the index will refuse to load it or check it out to the filesystem (and even if we did not have that safety, it would look like we were overwriting an untracked directory). For the same reason, it is difficult to create such a tree with regular git. Let's warn about these dubious entries during fsck, just in case somebody has created a bogus tree (and this also lets us prevent them from propagating when transfer.fsckObjects is set). Signed-off-by: Jeff King p...@peff.net --- I don't think this is happening in the wild, but I did see somebody playing around with libgit2 make such a tree (and it is easy to do with git-mktree, of course). Technically one could use git with such a tree as long as you never ever checked out the result, but I think it is sufficiently crazy that we should probably detect it, just in case. Can we declare . and .. illegal? There's no room for extension in tree objects and I'm thinking of using maybe . entry as an extension indicator. Not sure if it works, old gits may attempt to checkout . entries and fail... -- Duy -- 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: Python extension commands in git - request for policy change
On Sun, Nov 25, 2012 at 5:44 PM, Michael Haggerty mhag...@alum.mit.edu wrote: On the contrary, there is *constant* traffic on the mailing list about incompatibilities between different shell implementations (sh, dash, bash, etc), not to mention those in other utilities (sed, grep, etc) that one is forced to work with in shell scripts. Compatibility is a *huge* pain when developing shell code for git. The fact that users typically don't encounter such problems is due to the hard work of POSIX lawyers on the mailing list correcting the compatibility errors of mortal programmers. I think we still are in the process of moving away from shell-based commands (not the shell interface), just not enough man power to do it fast. The only shell-based command with active development is git-submodule. So most shell PITA is in the test suite. The most important issues to consider when imagining a future with a hybrid of code in C and some scripting language X are: * Portability: is X available on all platforms targeted by git, in usable and mutually-compatible versions? * Startup time: Is the time to start the X interpreter prohibitive? (On my computer, python -c pass, which starts the Python interpreter and does nothing, takes about 24ms.) This overhead would be incurred by every command that is not pure C. * Should the scripting language access the C functionality only by calling pure-C executables or by dynamically or statically linking to a binary module interface? If the former, then the granularity of interactions between X and C is necessarily coarse, and X cannot be used to implement anything but the outermost layer of functionality. If the latter, then the way would be clear to implement much more of git in X (and lua would also be worth considering). * Learning curve for developers: how difficult is it for a typical git developer to become conversant with X, considering both (1) how likely is it that the typical git developer already knows X and (2) how straightforward and predictable is the language X? In this category I think that Python has a huge advantage over Perl, though certainly opinions will differ and Ruby would also be a contender. * We might also need an embedded language variant, like Jeff's lua experiment. I'd be nice if X can also take this role. -- Duy -- 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/11] alternative unify appending of sob
On Mon, Nov 26, 2012 at 8:45 AM, Brandon Casey draf...@gmail.com wrote: From: Brandon Casey bca...@nvidia.com I hate to have the battle of the patches, but I kinda prefer the append_signoff code in sequencer.c over the code in log-tree.c as a foundation to build on. So, this series is similar to Duy's unification series, but it goes in the opposite direction and builds on top of sequencer.c and additionally adds the elements from my original series to treat the (cherry picked from... line added by 'cherry-pick -x' in the same way that other footer elements are treated (after addressing Junio's comments about rfc2822 continuation lines and duplicate s-o-b's). I've integrated Duy's series with a few minor tweaks. I added a couple of additional tests to t4014 and corrected one of the tests which had incorrect behavior. I think his sign-off's should still be valid, so I kept them in. Sorry that I've been slow, and now the two of us are stepping on each other's toes, but Duy please take a look and let me know if there's anything you dislike. I'm still not sure whether format-patch should follow cherry-pick's rule in appending sob. If it does, it probably makes more sense to fix the sequencer.c code then delete log-tree.c (not fixes on log-tree.c then delete it). I see that your changes pass all the new tests I added in format-patch so sequencer.c is probably good enough, log-tree.c changes are probably not needed. Feel free take over the series :-) -- Duy -- 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: Topics currently in the Stalled category
On Wed, Nov 21, 2012 at 7:05 AM, Junio C Hamano gits...@pobox.com wrote: * nd/unify-appending-of-s-o-b (2012-11-15) 1 commit - Unify appending signoff in format-patch, commit and sequencer I am not sure if the logic to refrain from adding a sign-off based on the existing run of sign-offs is done correctly in this change. I'm not sure what's the right logic here. The original code in format-patch has exactly one test just to verify that --signoff does _something_. There's another signoff code in git-am.sh and we should make sure at least the behavior is consistent. I guess I should start over by writing new tests to record the current behavior, then fixes and finally the unification. I think you can drop this for now. -- Duy -- 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 3/4] pathspec: apply *.c optimization from exclude
On Tue, Nov 20, 2012 at 4:20 AM, Junio C Hamano gits...@pobox.com wrote:
$ time git rev-list --quiet HEAD -- '*.c'
real0m40.770s
user0m40.290s
sys 0m0.256s
With the patch
$ time ~/w/git/git rev-list --quiet HEAD -- '*.c'
real0m34.288s
user0m33.997s
sys 0m0.205s
The above command is not supposed to be widely popular.
Hrm, perhaps. I use git grep pattern -- \*.c quite a lot, but
haven't seen use case for \*.c in the context of the log family.
git diff *.c is also helpful (maybe git diff *.[ch] is more often
used). But I suspect I/O dominates in both grep and diff cases. I just
try to make sure matching code won't show up in profile.
+#define PSF_ONESTAR 1
Together with the GF_ prefix in the previous, PSF_ prefix needs a
bit of in-code explanation. Is it just an RC3L (random combination
of 3 letters?)
I'm pretty sure PS stands for pathspec. F is probably from
fnmatch. Any suggestions?
@@ -46,6 +46,12 @@ inline int git_fnmatch(const char *pattern, const char
*string,
pattern += prefix;
string += prefix;
}
+ if (flags GF_ONESTAR) {
+ int pattern_len = strlen(++pattern);
+ int string_len = strlen(string);
+ return strcmp(pattern,
+ string + string_len - pattern_len);
+ }
What happens when pattern=foo*oob and string=foob?
The prefix match before this code determines that the literal prefix
in the pattern matches with the early part of the string, and makes
pattern=*oob and string=b.
When you come to strcmp(), you see that string_len is 1, pattern_len
is 3, and pattern is oob. string+string_len-pattern_len = oob,
one past the beginning of the original string foob. They match.
Oops?
Oops indead. I'll need to check exclude code too :(
return (string_len pattern_len) ||
strcmp(pattern, string + string_len - pattern_len);
perhaps?
Yeah.
--
Duy
--
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: Crash when pushing large binary file
On Fri, Nov 16, 2012 at 1:54 PM, Thomas Gay t...@tokyois.com wrote: If you set receive.unpacklimit to 1 on the receiving end, does it still crash? Yes. The crash log looks the same too. If it still says unpack-objects died of signal 11 then it was not done the right way. The receiving end can use either unpack-objects or index-pack for storing the objects. I know unpack-objects is not ready for large blobs (though I cannot explain your crash log, that's why I still need you to test it this way). I was hoping to force it use index-pack and see it still crashes. If it does, we have other problems than unpack-objects not being ready for large blobs. If it does not, I'd say it's a known issue with a known solution (I was planning on merging unpack-objects functionality back to index-pack). We can try again this way. index-pack will be used if the number of transfer objects exceeds 100 (by default). You are pusing 16 objects, which is why unpack-objects is used. We can try to push garbage to the other end to meet the 100 limit, then reset the branch at the other end later. You can run git gc early on the other end to clean up garbage, or it'll be done automatically at some point in future. Make sure there is no changes in index and worktree, or adjust you may want to change the last four commands slightly. mkdir tmp for i in `seq 200`;do echo $i tmp/$i; git add $i; done git commit -m 'useless stuff' git push where? # should not crash again git reset --hard HEAD^ git push same-where?-above -- Duy -- 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: [regression] Newer gits cannot clone any remote repos
On Wed, Nov 14, 2012 at 2:55 AM, Douglas Mencken dougmenc...@gmail.com wrote: Could you try re-building git with the NO_THREAD_SAFE_PREAD build variable set? Yeah! It works!!! --- evil/Makefile +++ good/Makefile @@ -957,6 +957,7 @@ HAVE_PATHS_H = YesPlease LIBC_CONTAINS_LIBINTL = YesPlease HAVE_DEV_TTY = YesPlease + NO_THREAD_SAFE_PREAD = YesPlease endif ifeq ($(uname_S),GNU/kFreeBSD) NO_STRLCPY = YesPlease With this, I do have correctly working git clone. Sorry you had to figure that out the hard way. Could you make it a proper patch? I'm surprised that Linux pread does not behave the same way across platforms though. Or maybe it only happens with certain Linux versions. What version are you using? -- Duy -- 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] Unify appending signoff in format-patch, commit and sequencer
On Fri, Nov 16, 2012 at 3:42 AM, Brandon Casey draf...@gmail.com wrote: Interestingly this patch triggers the fault that it fixes. I was surprised that there was no blank line before my S-o-b and thought I broke something. It turns out I used unmodified format-patch and it mistook the S-o-b quote as true S-o-b line. The modified one puts the blank line back. Heh, yeah I noticed this bug yesterday when I submitted my changes affecting the same area of code (sequence.c). Glad I didn't waste too much time fixing it. Have you looked at this: http://thread.gmane.org/gmane.comp.version-control.git/209781 That series doesn't duplicate your work, but the two series's should be resolved. Thanks I'm watching that discussion now and probably will rebase on top of yours once it's finalized. I actually wrote this patch a while ago, just waiting for nd/builtin-to-libgit to graduate because of some linking issues this patch causes. I can wait a bit longer until yours graduates. -- Duy -- 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] wildmatch: correct isprint and isspace
On Fri, Nov 16, 2012 at 12:13 AM, Jan H. Schönherr schn...@cs.tu-berlin.de wrote: #define isprint(x) (sane_istest(x, GIT_ALPHA | GIT_DIGIT | GIT_SPACE | \ GIT_PUNCT | GIT_REGEX_SPECIAL | GIT_GLOB_SPECIAL | \ - GIT_PATHSPEC_MAGIC)) + GIT_PATHSPEC_MAGIC) \ + (x) = 32) May I suggest the current is_print() implementation in master: #define isprint(x) ((x) = 0x20 (x) = 0x7e) To summarize my opinion: I no longer see a reason to correct isspace() (unless somebody with an actual use case complains), and a more POSIXly isprint() is already in master. = Nothing to do. :) Yeah. I remember to remind myself to check the implementation in master you mentioned but I probably failed at that. Just checked that isprint() is already in master, and your comment about isspace() use in wildmatch.c makes sense too. So I'm all for doing nothing. -- Duy -- 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: Crash when pushing large binary file
On Fri, Nov 16, 2012 at 12:44 PM, Thomas Gay t...@tokyois.com wrote: Using Git 1.8 on Mac OS X 10.7.5. I just added a large binary file to How large exactly? my repo, and each time I try to push it, Git crashes. I've attached the crash log to this email and pasted the console output below. Counting objects: 27, done. Delta compression using up to 4 threads. Compressing objects: 100% (16/16), done. error: unpack-objects died of signal 11 | 76.91 MiB/s error: pack-objects died of signal 13 error: failed to push some refs to 'ssh://...' If you set receive.unpacklimit to 1 on the receiving end, does it still crash? -- Duy -- 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: Reviews on mailing-list
On Mon, Nov 12, 2012 at 4:15 AM, David Lang da...@lang.hm wrote: Using a web browser requires connectivity at the time you are doing the review. Mailing list based reviews can be done at times when you don't have connectivity. I am not against email-based reviews but I'd like to point out that with Google Gears (and HTML5 Storage?) Gerrit can be made work offline too. I don't know how much work required though. -- Duy -- 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: Rename edge case...
On Fri, Nov 9, 2012 at 5:25 PM, John Szakmeister j...@szakmeister.net wrote: On Fri, Nov 9, 2012 at 4:27 AM, Tomas Carnecky tomas.carne...@gmail.com wrote: [snip] When merging two branches, git only looks at the tips. It doesn't inspect their histories to see how the files were moved around. So i doesn't matter whether you rename the files in a single commit or multiple commits. The resulting tree is always the same. I guess I figured that when I saw the final result, but didn't know if there was a way to coax Git into doing a better job here. I guess not though. There is not (yet). There were a few patches around that allow users to override git's automatic renames. You can search the mail archive. I think the keywords are rename cache. Thanks for reminding me for putting a higher priority on that. -- Duy -- 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: Revert option for git add --patch
On Thu, Nov 8, 2012 at 3:57 PM, Jonathon Mah j...@me.com wrote: Nathan, I find myself performing similar actions to you: using git add -p to stage hunks, sometimes editing the staged patch; and keeping mental notes of things I wanted to revert, sometimes changing them in the editor in another window, and sometimes reverting them after the add session with git checkout -p). I agree. I'd be really nice to have an interactive command about hunk manipulation between HEAD (or some other ref), index and worktree in both directions (except writing to HEAD, of course). This reminds me of the git put discussion a while ago, which also moves code between different locations.. -- Duy -- 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: Bug: write-tree corrupts intent-to-add index state
On Tue, Nov 06, 2012 at 12:53:27AM -0800, Jonathon Mah wrote: It appears the index forgot that those files were new. So not only has the intent-to-add status been lost, but git status shows a file existing in neither HEAD nor the index as not-new-but-modified. Tracing through it, I narrowed it down to git write-tree affecting the index state. As far as I can tell, it's incorrect for that command to change the index. I'm now out of my (shallow) depth in the git codebase, so I'm throwing it out there for someone to tackle (hopefully). I've attached a failing test case. I played with your test a bit and found that if we skip the commit step, the bug disappears. I checked and believe that i-t-a flag in index is preserved, which leaves cache-tree as the culprit. If cache-tree is (incorrectly) valid, diff operations won't bother checking index. We used to not allow writing tree with i-t-a entries present. Then we allowed it, but forgot that we need to invalidate any paths that contain i-t-a entries, otherwise cache-tree won't truly reflect index. The below patch seems to do the trick, but I'd rather do the invalidation inside update_one() so we don't need to traverse over the index again. I haven't succesfully put cache-tree.c back in my head again to make it happen. Anybody is welcome to step up, or I'll finish it this weekend. -- 8 -- diff --git a/cache-tree.c b/cache-tree.c index 28ed657..30a8018 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -381,6 +381,9 @@ int cache_tree_update(struct cache_tree *it, i = update_one(it, cache, entries, , 0, flags); if (i 0) return i; + for (i = 0; i entries; i++) + if (cache[i]-ce_flags CE_INTENT_TO_ADD) + cache_tree_invalidate_path(it, cache[i]-name); return 0; } diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh index ec35409..3ddbd89 100755 --- a/t/t2203-add-intent.sh +++ b/t/t2203-add-intent.sh @@ -62,5 +62,25 @@ test_expect_success 'can commit -a with an i-t-a entry' ' git commit -a -m all ' +test_expect_success 'cache-tree invalidates i-t-a paths' ' + git reset --hard + mkdir dir + : dir/foo + git add dir/foo + git commit -m foo + + : dir/bar + git add -N dir/bar + git diff --cached --name-only actual + echo dir/bar expect + test_cmp expect actual + + git write-tree /dev/null + + git diff --cached --name-only actual + echo dir/bar expect + test_cmp expect actual +' + test_done -- 8 -- -- 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 14/13] wildmatch: fix tests that fail on Windows due to path mangling
On Mon, Nov 5, 2012 at 4:00 AM, Johannes Sixt j...@kdbg.org wrote: Patterns beginning with a slash are converted to Windows paths before test-wildmatch gets to see them. Use a different first character. Or we could prepend the paths with something, which is then cut out by test-wildmatch. Not sure if it's intuitive to look at the tests though. After this change, there are still 3 failing tests that are in connection with [[:xdigit:]]. Don't know, yet, what's going on there. the wildmatch tests or fnmatch ones? -- Duy -- 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: relative objects/info/alternates doesn't work on remote SMB repo
On Tue, Oct 30, 2012 at 4:28 PM, Orgad Shaneh Any news? This still doesn't work with 1.8.0. Nope, sorry. It's still in my todo list. -- Duy -- 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: Strange behaviour of git diff branch1 ... branch2
On Sat, Oct 27, 2012 at 7:33 PM, Junio C Hamano gits...@pobox.com wrote: Nguyen Thai Ngoc Duy pclo...@gmail.com wrote: Notice the --cc in the first line, which is combined diff. Usually combined-diff is between two points and one parent. Though somehow git passes 4 parents down combined-diff.c:show_combined_header, as you can see in the index line. I think we should fix rev parsing code as it does not make sense to pass 4 identical parents this way. The two heads home from HEAD...HEAD the user has on the command line. The user is getting exactly what she asked; there is nothing to fix. Is there any use case where HEAD...HEAD (or ... alone) is actually useful? I have re-read the git-diff man page and I don't think it explains git diff foo ... bar syntax (from a user's point of view, not a git guru's). We could improve the documentation if git diff foo ... bar is useful, or reject it with an error to avoid confusion. -- Duy -- 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: Strange behaviour of git diff branch1 ... branch2
On Sat, Oct 27, 2012 at 4:26 AM, Aaron Schrab aa...@schrab.com wrote: I came across this odd question on stackoverflow: http://stackoverflow.com/q/13092854/1507392 If git diff is run with ... as a separate argument between two commit-ish arguments causes it to produce strange output. The differences seem to be the same as if ... was left out, but change lines begin with 4 + or - characters rather than just 1. Can anybody explain what is happening here? I don't have any reason to want to use that form myself, but I'm very curious about why it produces this odd output. I'm curious too. It shows this to me diff --cc .gitignore index a188a82,a188a82,a188a82,a188a82..d4473d8 --- a/.gitignore +++ b/.gitignore @ -2,9 -2,9 -2,9 -2,9 +2,6 @ /GIT-CFLAGS /GIT-LDFLAGS /GIT-GUI-VARS /GIT-PREFIX /GIT-SCRIPT-DEFINES /GIT-USER-AGENT /GIT-VERSION-FILE /bin-wrappers/ /git Notice the --cc in the first line, which is combined diff. Usually combined-diff is between two points and one parent. Though somehow git passes 4 parents down combined-diff.c:show_combined_header, as you can see in the index line. I think we should fix rev parsing code as it does not make sense to pass 4 identical parents this way. -- Duy -- 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] Move try_merge_command and checkout_fast_forward to libgit.a
On Thu, Oct 25, 2012 at 4:45 PM, Jeff King p...@peff.net wrote: On Tue, Oct 23, 2012 at 09:24:51AM +0700, Nguyen Thai Ngoc Duy wrote: These functions are called in sequencer.c, which is part of libgit.a. This makes libgit.a potentially require builtin/merge.c for external git commands. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- I made some unrelated changes in sequencer.c and this problem shown up. merge-recursive.c is probably not the best place for these functions. I just don't want to create merge.c for them. I'm fine with this conceptually, but merge-recursive really is the wrong place. I'd much rather see a new merge.c to collect merge-related helper functions that are not strategy-specific. OK. I checked around for similar issues and found these used by libgit.a but stay in builtin/ instead: estimate_bisect_steps: bisect.c and builtin/rev-list.c print_commit_list: bisect.c and builtin/rev-list.c - move them to bisect.c? another candidate is revision.c. fetch_pack: transport.c and builtin/fetch-pack.c send_pack: transport.c and builtin/send-pack.c - move them to transport.c? or new files fetch-pack.c and send-pack.c? I haven't check how many functions they may pull together. setup_diff_pager: diff-no-index.c and builtin/diff.c - to diff-lib.c? -- Duy -- 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: Large number of object files
On Wed, Oct 24, 2012 at 12:21 PM, Uri Moszkowicz u...@4refs.com wrote: Continuing to work on improving clone times, using git gc --aggressive has resulted in a large number of tags combining into a single file but now I have a large number of files in the objects directory - 131k for a ~2.7GB repository. Can you paste git count-objects -v? I'm curious why gc keeps so many loose objects around. Any way to reduce the number of these files to speed up clones? An easy way to get rid of them is to clone the non-local way. Everything will be sent over a pack, the result would be a single pack in new repo. Try git clone file:///path/to/source/repo new-repo. You can also try git prune on the existing repo (read its man page before use). -- Duy -- 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: Long clone time after done.
On Wed, Oct 24, 2012 at 1:30 AM, Uri Moszkowicz u...@4refs.com wrote: I have a large repository which I ran git gc --aggressive on that I'm trying to clone on a local file system. I would expect it to complete very quickly with hard links but it's taking about 6min to complete with no checkout (git clone -n). I see the message Clining into 'repos'... done. appear after a few seconds but then Git just hangs there for another 6min. Any idea what it's doing at this point and how I can speed it up? done. is printed by clone_local(), which is called in cmd_clone(). After that there are just a few more calls. Maybe you could add a few printf in between these calls, see which one takes most time? -- Duy -- 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: In search of a version control system
On Sun, Oct 21, 2012 at 7:20 PM, Drew Northup n1xim.em...@gmail.com wrote: On Tue, Oct 9, 2012 at 1:58 AM, Matthieu Moy matthieu@grenoble-inp.fr wrote: David Aguilar dav...@gmail.com writes: I would advise against the file locking, though. You ain't gonna need it ;-) What do you suggest to merge Word files? If the files are in the DOCX format you can just expand them as zip archives and diff what's inside of them. The text in particular is stored as XML. An XML diff means nothing to a regular user, and manually merging this way can produce unreadable Words files. So no, it's still not an answer. -- Duy -- 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] Cache stat_tracking_info() for faster status and branch -v
On Sat, Oct 20, 2012 at 2:50 AM, Junio C Hamano gits...@pobox.com wrote: Not particularly interested in the cause, but not so strongly against it to veto it. I wonder how many people keep old branches like I do, which are usually far from remotes. Doesn't it make more sense to use a notes-cache that is keyed off of the commit object name X of the remote? You will have a single note that stores a blob for the commit object remotes/origin/master, and the blob tells you how far the commit at the tip of 'frotz' is from it, and the same for 'xyzzy'. You would obviouly need to run gc on such a notes-cache tree from time to time, removing notes for commits that are not tip of any branch that could be a fork point, and from the remaining notes blobs, entries that describe commits that are not tip of any branch, if you go that route. The notes-cache route looks much nicer. Thanks. We can also use Jeff's persistent hash table from his rename-cache series. -- Duy -- 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 (Oct 2012, #06; Fri, 19)
On Sat, Oct 20, 2012 at 4:03 AM, Junio C Hamano gits...@pobox.com wrote: * nd/wildmatch (2012-10-15) 13 commits Allows pathname patterns in .gitignore and .gitattributes files with double-asterisks foo/**/bar to match any number of directory hierarchies. I suspect that this needs to be plugged to pathspec matching code; otherwise git log -- 'Docum*/**/*.txt' would not show the log for commits that touch Documentation/git.txt, which would be confusing to the users. I do want non-recursive * in pathspec and ** can help retain the recursive * semantics. But can we just flip the coin at some point and change * semantics in pathspec from recursive to non-recursive? I have no problem with that but then we might want to change other places that use fnmatch without FNM_PATHNAME too, e.g. apply, branch, for-each-ref... Or we could go with new syntax :(glob)Docu*/**/*.txt. A bit ugly. -- Duy -- 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/6] Bring format-patch --notes closer to a real feature
On Thu, Oct 18, 2012 at 12:45 PM, Junio C Hamano gits...@pobox.com wrote: This replaces the earlier wip with a real thing. We never advertised the --notes option to format-patch (or anything related to the pretty format options for that matter) because the behaviour of these options was whatever they happened to do, not what they were designed to do. Stupid question: does git am recreate notes from format-patch --notes output? If it does not, should it? I think it could be a nice way of copying notes from one machine to another, but not enabled by default (iow am --notes). -- Duy -- 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: Unexpected directories from read-tree
On Fri, Oct 19, 2012 at 6:10 AM, Uri Moszkowicz u...@4refs.com wrote: I'm testing out the sparse checkout feature of Git on my large (14GB) repository and am running into a problem. When I add dir1/ to sparse-checkout and then run git read-tree -mu HEAD I see dir1 as expected. But when I add dir2/ to sparse-checkout and read-tree again I see dir2 and dir3 appear and they're not nested. If I replace dir2/ with dir3/ in the sparse-checkout file, then I see dir1 and dir3 but not dir2 as expected again. How can I debug this problem? Posting here is step 1. What version are you using? You can look at unpack-trees.c The function that does the check is excluded_from_list. You should check ls-files -t, see if CE_SKIP_WORKTREE is set correctly for all dir1/*, dir2/* and dir3/*. Can you recreate a minimal test case for the problem? -- Duy -- 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: L10n regression in 1.8.0.rc2: diffstat summary (git diff --stat, git format-patch)
On Wed, Oct 17, 2012 at 5:53 PM, Peter Krefting pe...@softwolves.pp.se wrote: Hi! The output of git format-patch and git diff --stat no longer becomes localized when using 1.8.0.rc2, compared to 1.7.12 It's the result of this discussion [1]. I don't remember exactly the open issues. But I think it involves drawing a line between team language vs local language, whether team language can be anything other than English, the maintenance cost for supporting it. You're welcome to revive the discussion. Maybe we can find a solution that is agreed by all parties. [1] http://thread.gmane.org/gmane.comp.version-control.git/204285/focus=204283 -- Duy -- 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 v5 02/12] ctype: support iscntrl, ispunct, isxdigit and isprint
On Wed, Oct 17, 2012 at 7:09 PM, Jan H. Schönherr
schn...@cs.tu-berlin.de wrote:
const unsigned char sane_ctype[256] = {
- 0, 0, 0, 0, 0, 0, 0, 0, 0, S, S, 0, 0, S, 0, 0, /* 0.. 15 */
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, /* 16.. 31 */
+ X, X, X, X, X, X, X, X, X, Z, Z, X, X, Z, X, X, /* 0.. 15 */
+ X, X, X, X, X, X, X, X, X, X, X, X, X, X, X, X, /* 16.. 31 */
Normal isspace() also includes vertical tab (11) and form-feed (12) as
white-space characters. Is there a reason, why they are not included here?
I'm not sure. They were not classified as spaces in the very first
version in 4546738 (Unlocalized isspace and friends - 2005-10-13).
Maybe Linus had a reason to do so.
+#define isprint(x) (sane_istest(x, GIT_ALPHA | GIT_DIGIT | GIT_SPACE | \
+ GIT_PUNCT | GIT_REGEX_SPECIAL | GIT_GLOB_SPECIAL | \
+ GIT_PATHSPEC_MAGIC))
Normal isprint() only includes space (32) from the white-space characters.
The other white-space characters are not considered printable.
Do we want to stay close to the original, or not?
We do. I followed [1] but obvious missed the last sentence in print
description: No characters specified for the keyword cntrl shall be
specified. Thanks for catching. I'll fix it soon.
[1] http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap07.html
--
Duy
--
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 12/12] Add git-check-ignore sub-command
Adam, do you have time to continue this series? I can help polish it for inclusion, but I don't want to step in your way if you are quietly updating it. On Tue, Oct 16, 2012 at 5:31 AM, Junio C Hamano gits...@pobox.com wrote: + +If `-z` is specified, the output is a series of lines of the form: + Hmph... the remainder of the paragraph seems to have been chopped off. No, the last version [1] looks the same. I was worried there was a bug somewhere in the toolchain that chopped it off. [1] http://thread.gmane.org/gmane.comp.version-control.git/204661/focus=206085 +EXIT STATUS +--- + +0:: + One or more of the provided paths is ignored. -- Duy -- 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 12/12] Add git-check-ignore sub-command
On Tue, Oct 16, 2012 at 9:09 PM, Adam Spiers g...@adamspiers.org wrote: I was *intending* to finish it off soon, but I have been really busy with work and other commitments recently, which has prevented this. I don't currently have any unpublished changes which would conflict with your recent work, and I'm at a conference this week, so feel free to carry on polishing if you want. However I will probably have some responses on the discussion about current issues, so it would be good if I was given a chance to catch up on this discussion before the series makes its way to master. No hurry. And thanks for your contribution. -- Duy -- 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: push race
On Mon, Oct 15, 2012 at 6:05 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Angelo Borsotti angelo.borso...@gmail.com writes: the push command checks first if the tips of the branches match those of the remote references, and if it does uploads the snapshot. The update does two things: upload objects to the database, and then update the reference. Adding objects to the database does not change the repository until the objects are reachable from a ref. Updating the ref is usually done giving the expected old sha1, and locks the ref, so it can't change in the meantime. I don't know this part of the code very well, but check refs.c for the C part, and git update-ref for the plumbing interface. I think it's lock_any_ref_for_update(), which is called inside refs.c:update_ref(). -- Duy -- 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 v5 04/12] wildmatch: remove unnecessary functions
On Sun, Oct 14, 2012 at 12:04 PM, Junio C Hamano gits...@pobox.com wrote: Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- The comment-fix seems to be new but otherwise this is unchanged, right? Right.-- Duy -- 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 v5 02/12] ctype: support iscntrl, ispunct, isxdigit and isprint
On Sun, Oct 14, 2012 at 7:59 PM, René Scharfe
rene.scha...@lsrfire.ath.cx wrote:
+const unsigned char sane_ctype2[256] = {
+ CN, CN, CN, CN, CN, CN, CN, CN, CN, CN, CN, CN, CN, CN, CN, CN, /*
0..15 */
+ CN, CN, CN, CN, CN, CN, CN, CN, CN, CN, CN, CN, CN, CN, CN, CN, /*
16..31 */
+ 0, PU, PU, PU, PU, PU, PU, PU, PU, PU, PU, PU, PU, PU, PU, PU, /*
32..47 */
+ XD, XD, XD, XD, XD, XD, XD, XD, XD, XD, PU, PU, PU, PU, PU, PU, /*
48..63 */
+ PU, 0, XD, 0, XD, 0, XD, 0, 0, 0, 0, 0, 0, 0, 0, 0, /*
64..79 */
+ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, PU, PU, PU, PU, PU, /*
80..95 */
+ PU, 0, XD, 0, XD, 0, XD, 0, 0, 0, 0, 0, 0, 0, 0, 0, /*
96..111 */
+ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, PU, PU, PU, PU, CN, /*
112..127 */
Shouldn't [ace] (65, 67, 69) and [ACE] (97, 99, 101) be xdigits as well?
Hmm.. I generated it from LANG=C. I wonder where I got it wrong..
But how about using the existing hexval_table instead, like this:
#define isxdigit(x) (hexval_table[(x)] != -1)
With that, couldn't you squeeze the other two classes into the existing
sane_type?
No there are still conflicts: 9, 10 and 13 as spaces (vs controls) and
123, 124 and 126 as regex/pathspec special (vs punctuation).
By the way, I'm working on a patch series for implementing a lot more
character classes with table lookups. It grew out of a desire to make
bad_ref_char() faster but perhaps got a bit out of hand by now; it's at 24
patches and still not finished. I'm curious how long we have until it
escapes. ;-)
I don't think the series is going to graduate any time soon :)
--
Duy
--
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 v5 02/12] ctype: support iscntrl, ispunct, isxdigit and isprint
On Sun, Oct 14, 2012 at 03:59:31PM +0200, René Scharfe wrote:
Am 14.10.2012 15:25, schrieb Nguyen Thai Ngoc Duy:
On Sun, Oct 14, 2012 at 7:59 PM, René Scharfe
rene.scha...@lsrfire.ath.cx wrote:
With that, couldn't you squeeze the other two classes into the existing
sane_type?
No there are still conflicts: 9, 10 and 13 as spaces (vs controls) and
123, 124 and 126 as regex/pathspec special (vs punctuation).
That's not a problem, an entry in the table can have more than one bit
set -- just OR them together in ctype.c. It may not look as nice, but
that's OK. You could also define a character for GIT_SPACE | GIT_CNTRL
etc. for cosmetic reasons.
Only space chars is not a subset of control chars, which needs a new
combination. So the result does not look as bad as I thought:
-- 8 --
diff --git a/ctype.c b/ctype.c
index faeaf34..0bfebb4 100644
--- a/ctype.c
+++ b/ctype.c
@@ -11,18 +11,21 @@ enum {
D = GIT_DIGIT,
G = GIT_GLOB_SPECIAL, /* *, ?, [, \\ */
R = GIT_REGEX_SPECIAL, /* $, (, ), +, ., ^, {, | */
- P = GIT_PATHSPEC_MAGIC /* other non-alnum, except for ] and } */
+ P = GIT_PATHSPEC_MAGIC, /* other non-alnum, except for ] and } */
+ X = GIT_CNTRL,
+ U = GIT_PUNCT,
+ Z = GIT_CNTRL | GIT_SPACE
};
const unsigned char sane_ctype[256] = {
- 0, 0, 0, 0, 0, 0, 0, 0, 0, S, S, 0, 0, S, 0, 0, /* 0.. 15 */
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, /* 16.. 31 */
+ X, X, X, X, X, X, X, X, X, Z, Z, X, X, Z, X, X, /* 0.. 15 */
+ X, X, X, X, X, X, X, X, X, X, X, X, X, X, X, X, /* 16.. 31 */
S, P, P, P, R, P, P, P, R, R, G, R, P, P, R, P, /* 32.. 47 */
D, D, D, D, D, D, D, D, D, D, P, P, P, P, P, G, /* 48.. 63 */
P, A, A, A, A, A, A, A, A, A, A, A, A, A, A, A, /* 64.. 79 */
- A, A, A, A, A, A, A, A, A, A, A, G, G, 0, R, P, /* 80.. 95 */
+ A, A, A, A, A, A, A, A, A, A, A, G, G, U, R, P, /* 80.. 95 */
P, A, A, A, A, A, A, A, A, A, A, A, A, A, A, A, /* 96..111 */
- A, A, A, A, A, A, A, A, A, A, A, R, R, 0, P, 0, /* 112..127 */
+ A, A, A, A, A, A, A, A, A, A, A, R, R, U, P, X, /* 112..127 */
/* Nothing in the 128.. range */
};
diff --git a/git-compat-util.h b/git-compat-util.h
index f8b859c..db77f3e 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -510,6 +510,10 @@ extern const char tolower_trans_tbl[256];
#undef isupper
#undef tolower
#undef toupper
+#undef iscntrl
+#undef ispunct
+#undef isxdigit
+#undef isprint
extern const unsigned char sane_ctype[256];
#define GIT_SPACE 0x01
#define GIT_DIGIT 0x02
@@ -517,6 +521,8 @@ extern const unsigned char sane_ctype[256];
#define GIT_GLOB_SPECIAL 0x08
#define GIT_REGEX_SPECIAL 0x10
#define GIT_PATHSPEC_MAGIC 0x20
+#define GIT_CNTRL 0x40
+#define GIT_PUNCT 0x80
#define sane_istest(x,mask) ((sane_ctype[(unsigned char)(x)] (mask)) != 0)
#define isascii(x) (((x) ~0x7f) == 0)
#define isspace(x) sane_istest(x,GIT_SPACE)
@@ -527,6 +533,13 @@ extern const unsigned char sane_ctype[256];
#define isupper(x) sane_iscase(x, 0)
#define is_glob_special(x) sane_istest(x,GIT_GLOB_SPECIAL)
#define is_regex_special(x) sane_istest(x,GIT_GLOB_SPECIAL | GIT_REGEX_SPECIAL)
+#define iscntrl(x) (sane_istest(x,GIT_CNTRL))
+#define ispunct(x) sane_istest(x, GIT_PUNCT | GIT_REGEX_SPECIAL | \
+ GIT_GLOB_SPECIAL | GIT_PATHSPEC_MAGIC)
+#define isxdigit(x) (hexval_table[x] != -1)
+#define isprint(x) (sane_istest(x, GIT_ALPHA | GIT_DIGIT | GIT_SPACE | \
+ GIT_PUNCT | GIT_REGEX_SPECIAL | GIT_GLOB_SPECIAL | \
+ GIT_PATHSPEC_MAGIC))
#define tolower(x) sane_case((unsigned char)(x), 0x20)
#define toupper(x) sane_case((unsigned char)(x), 0)
#define is_pathspec_magic(x) sane_istest(x,GIT_PATHSPEC_MAGIC)
-- 8 --
--
Duy
--
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] exclude: fix a bug in prefix comparison optimization
On Mon, Oct 15, 2012 at 12:36 AM, Junio C Hamano gits...@pobox.com wrote: With your teach attr.c match the same optimization as dir.c series, you would need something like this diff --git i/attr.c w/attr.c index 6d39406..528e935 100644 --- i/attr.c +++ w/attr.c @@ -710,7 +710,7 @@ static int path_matches(const char *pathname, int pathlen, * if the non-wildcard part is longer than the remaining * pathname, surely it cannot match. */ - if (!namelen || prefix namelen) + if (prefix namelen) return 0; if (baselen != 0) baselen++; If there's still a chance to rewrite attr-match-optim-more series (I see it's in next now), then I could reorder the patches so that excluded_from_list code refactoring goes first, then rewrite teach attr.c match... as dir.c patch makes use of the new functions without code duplication. The end result would be the same, except that we won't see this bug in attr.c's history. Not much value so if it may take a lot of your time, don't bother. -- Duy -- 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] t0003-attributes.sh: Don't overwrite t/.gitattributes
On Sun, Oct 14, 2012 at 12:44 AM, Ramsay Jones ram...@ramsay1.demon.co.uk wrote: Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk --- Hi Junio, This test in the current pu branch (commit bb0e6bf Merge branch 'aw/rebase-am-failure-detection' into pu, 11-10-2012) overwrites the contents of t/.gitattributes. Note that the merge of the two branches 'nd/wildmatch' and 'nd/attr-match-optim-more' both add new tests at the end, and *both* sets initially 'cd ..' ... I'm resending nd/wildmatch to move the tests before bare repo tests so cd .. is no longer necessary. nd/attr-match-optim-more has the same fix. t/t0003-attributes.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh index 7ed288f..fe80af7 100755 --- a/t/t0003-attributes.sh +++ b/t/t0003-attributes.sh @@ -247,7 +247,6 @@ test_expect_success 'patterns starting with exclamation' ' ' test_expect_success '** test' ' - cd .. echo **/f foo=bar .gitattributes cat \EOF expect f: foo: bar -- 1.7.12 -- Duy -- 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 v5 02/12] ctype: support iscntrl, ispunct, isxdigit and isprint
On Sun, Oct 14, 2012 at 12:02 PM, Junio C Hamano gits...@pobox.com wrote: Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- The description to justify why it is ctype2[] seems to have been lost. Intended? Nope. I added the description after generating patches and forgot to update the same to my branch. Thanks for catching. -- Duy -- 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 12/12] t3070: disable two fnmatch tests that have different results on different libc
On Fri, Oct 12, 2012 at 2:22 PM, Johannes Sixt j.s...@viscovery.net wrote: Am 10/10/2012 12:40, schrieb Nguyễn Thái Ngọc Duy: fnmatch on glibc-2.12.1 returns no match. glibc-2.15 returns ok. There are many more cases that fail with the fnmatch() that we ship in compat/fnmatch. To test this on Linux, you have to remove the #if defined _LIBC || !defined __GNU_LIBRARY__ brackets from compat/fnmatch/fnmatch.c and build with NO_FNMATCH. Thanks. The point of checking against fnmatch is to make sure that wildmatch does not divert in behavior from fnmatch. For some corner cases that behavior is undefined, I think it is ok for different fnmatch versions to behave differently. But some of failed tests make me worry about fnmatch. compat/fnmatch for example does not match '**/foo' with '/foo' (it does '*/foo' with '/foo'). 'A' is matched with '[[:digit:][:space:][:upper:]]' but not '[[:digit:][:upper:][:space:]]'. Perhaps we better off convert git to use wildmatch only to keep matching behavior more reliable. wildmatch does not support non-FNM_PATHNAME mode, but that should be easy to fix. The only downside I see is fnmatch may support locale while wildmatch does not. -- Duy -- 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 2/2] attr: more matching optimizations from .gitignore
On Thu, Oct 11, 2012 at 3:03 AM, Junio C Hamano gits...@pobox.com wrote: It would save time from both of us if you can check what is queued on 'pu'. I do not think I touched the code for off-by-one bugs there, though. 'pu' looks good. -- Duy -- 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 00/12] Wildmatch v4
On Fri, Oct 12, 2012 at 10:05:06AM -0700, Junio C Hamano wrote:
Nguyen, how about updating the match () shell function in 3070 so
that it not just says not-ok, but indicates what failed (wildmatch
failed, or wildmatch passed but fnmatch failed), at least when the
test is run as ./t3070-*.sh -v -i?
You could squash this to the Integrate wildmatch to git patch, or
just put it at the end of the series (I'll need to send a series
update anyway). This splits fnmatch and wildmatch tests separately so
we can easily identify which one fails.
-- 8 --
diff --git a/t/t3070-wildmatch.sh b/t/t3070-wildmatch.sh
index c3ee729..4f97923 100755
--- a/t/t3070-wildmatch.sh
+++ b/t/t3070-wildmatch.sh
@@ -5,20 +5,28 @@ test_description='wildmatch tests'
. ./test-lib.sh
match() {
-test_expect_success wildmatch $*
- if [ $1 = 1 ]; then
+if [ $1 = 1 ]; then
+ test_expect_success wildmatch:match $3 $4
test-wildmatch wildmatch '$3' '$4'
- else
+
+else
+ test_expect_success wildmatch: no match $3 $4
! test-wildmatch wildmatch '$3' '$4'
- fi
- if [ $2 = 1 ]; then
+
+fi
+if [ $2 = 1 ]; then
+ test_expect_success fnmatch: match $3 $4
test-wildmatch fnmatch '$3' '$4'
- elif [ $2 = x ]; then
- true
- else
+
+elif [ $2 = 0 ]; then
+ test_expect_success fnmatch: no match $3 $4
! test-wildmatch fnmatch '$3' '$4'
- fi
-
+
+#else
+# test_expect_success BROKEN_FNMATCH fnmatch: $3 $4
+# test-wildmatch fnmatch '$3' '$4'
+#
+fi
}
# Basic wildmat features
-- 8 --
--
Duy
--
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 2/2] attr: more matching optimizations from .gitignore
On Fri, Oct 12, 2012 at 12:09:57PM -0700, Junio C Hamano wrote: This is not entirely your fault, but please don't do that cd ... The original test had cd bare, made an assumption that step will never fail (which is mostly correct), and ran everything afterward in that subdirectory. Adding Do a 'cd ..' to come back is a horrible way to build on it. Imagine what happens when another person also did the same thing, and both changes need to be merged. You will end up going up two levels, which is not what you want. I think the right fix is to make each of the test that wants to run in bare chdir in its own subshell, or not append these new tests that do not run in the bare to the end of this file, but before the execution goes down to bare. The reason I put these tests at the end was because I destroy .gitattributes and it might affect the following tests. But obviously the bare tests run in its own repository (and I have not committed anything till the repo is cloned) so my .gitattributes changes can't affect them. Please squash this in -- 8 -- diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh index 4a1402f..f6c21ea 100755 --- a/t/t0003-attributes.sh +++ b/t/t0003-attributes.sh @@ -206,6 +206,16 @@ test_expect_success 'root subdir attribute test' ' attr_check subdir/a/i unspecified ' +test_expect_success 'negative patterns' ' + echo !f test=bar .gitattributes + test_must_fail git check-attr test -- f +' + +test_expect_success 'patterns starting with exclamation' ' + echo \!f test=foo .gitattributes + attr_check !f foo +' + test_expect_success 'setup bare' ' git clone --bare . bare.git cd bare.git @@ -242,18 +252,4 @@ test_expect_success 'bare repository: test info/attributes' ' attr_check subdir/a/i unspecified ' -test_expect_success 'leave bare' ' - cd .. -' - -test_expect_success 'negative patterns' ' - echo !f test=bar .gitattributes - test_must_fail git check-attr test -- f -' - -test_expect_success 'patterns starting with exclamation' ' - echo \!f test=foo .gitattributes - attr_check !f foo -' - test_done -- 8 -- -- Duy -- 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 3/3] grep: stop looking at random places for .gitattributes
On Thu, Oct 11, 2012 at 2:04 PM, Michael Haggerty mhag...@alum.mit.edu wrote: Maybe I'm being too much of a purist, but I don't think that git should retroactively reinterpret history on its own initiative in a way that might not be correct (e.g., maybe your encoding changed from ASCII to Shift-JIS sometime in the past). It would be more appropriate for this to happen only if explicitly requested by the user. For example, why don't you override the incorrect historical attributes via .git/info/attributes? I think git-notes is a more appropriate place to correct these things. If the incorrect commits are pruned, their notes can also be pruned. No poluttion in $GIT_DIR/info/attr.. And i'm your side, no checking worktree without user's permission. -- Duy -- 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: push.default documented in man git-push?
On Fri, Oct 05, 2012 at 01:03:28PM -0700, Junio C Hamano wrote:
OK. I think I was surprised that some messages were controlled by
advice.* but gave no hints about that and I found that out by other
means. I'll check all the advice messages.
It's about the commands, not the documents. For example I had no idea
I could set advice.statusHints to turn off some advice. git status
does not say that those messages can be turned off.
OK, the messages are supposed to advise how to turn them off, so we
would want some code updates in that case.
Something like this? It turns out none of the advice messages says
anything about advice.*.
This patch makes many output more verbose/annoying, (which is good,
more motivation to turn advice off). On the other hand, if a user sees
a message telling her/him to turn advice.statusHints off, (s)he may
miss other helpful advice as it turns the whole set off. I don't know
if that is good or bad for newbies.
Tests are broken due to output changes. Will fix it if it may have a
chance of entering git.git.
There are two advise() calls in sequencer.c that we may want to
trigger with a new advice config, by the way.
-- 8 --
diff --git a/advice.c b/advice.c
index edfbd4a..5b78c01 100644
--- a/advice.c
+++ b/advice.c
@@ -70,7 +70,8 @@ int error_resolve_conflict(const char *me)
advise(_(Fix them up in the work tree,\n
and then use 'git add/rm file' as\n
appropriate to mark resolution and make a commit,\n
-or use 'git commit -a'.));
+or use 'git commit -a'.\n
+Set advice.resolveConflict to false to turn off this
message.));
return -1;
}
@@ -89,7 +90,9 @@ void detach_advice(const char *new_name)
state without impacting any branches by performing another
checkout.\n\n
If you want to create a new branch to retain commits you create, you
may\n
do so (now or later) by using -b with the checkout command again.
Example:\n\n
- git checkout -b new_branch_name\n\n;
+ git checkout -b new_branch_name\n
+ Set either advice.detachedHead to false to turn off this message\n\n
+;
fprintf(stderr, fmt, new_name);
}
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 781295b..a805961 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -699,7 +699,9 @@ static void suggest_reattach(struct commit *commit, struct
rev_info *revs)
_(
If you want to keep them by creating a new branch,
this may be a good time\nto do so with:\n\n
-git branch new_branch_name %s\n\n),
+git branch new_branch_name %s\n
+ Set either advice.detachedHead to false
+to turn off this message\n\n),
sha1_to_hex(commit-object.sha1));
}
diff --git a/builtin/commit.c b/builtin/commit.c
index a17a5df..5a4d85f 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -50,7 +50,8 @@ N_(Your name and email address were configured automatically
based\n
\n
After doing this, you may fix the identity used for this commit with:\n
\n
-git commit --amend --reset-author\n);
+git commit --amend --reset-author\n
+Set either advice.implicitIdentity to false to turn off this message\n);
static const char empty_amend_advice[] =
N_(You asked to amend the most recent commit, but doing so would make\n
diff --git a/builtin/merge.c b/builtin/merge.c
index 0ec8f0d..754f51e 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1227,14 +1227,18 @@ int cmd_merge(int argc, const char **argv, const char
*prefix)
*/
if (advice_resolve_conflict)
die(_(You have not concluded your merge (MERGE_HEAD
exists).\n
- Please, commit your changes before you can
merge.));
+ Please, commit your changes before you can
merge.\n
+ Set advice.resolveConflict to false
+ to turn off this message\n));
else
die(_(You have not concluded your merge (MERGE_HEAD
exists).));
}
if (file_exists(git_path(CHERRY_PICK_HEAD))) {
if (advice_resolve_conflict)
die(_(You have not concluded your cherry-pick
(CHERRY_PICK_HEAD exists).\n
- Please, commit your changes before you can
merge.));
+ Please, commit your changes before you can
merge.\n
+ Set advice.resolveConflict to false
+ to turn off this message\n));
else
die(_(You have not concluded your cherry-pick
(CHERRY_PICK_HEAD exists).));
}
diff --git a/builtin/push.c b/builtin/push.c
Re: [PATCH 1/3] quote: let caller reset buffer for quote_path_relative()
On Thu, Oct 11, 2012 at 4:13 AM, Junio C Hamano gits...@pobox.com wrote: This sounds a lot stronger than let to me. All existing callers that assumed that buf to be emptied by the function now has to clear it. quote: stop resetting output buffer of quote_path_relative() may better describe what this really does. How should this interact with the logic in the called function that used to say if we ended up returning an empty string because the path is the same as the base, we should give ./ back, and what should the return value of this function be? ... So what does it mean to have an existing string in the output buffer when calling the function? Is it supposed to be a path to a directory, or just a general uninterpreted string (e.g. a message)? I prefer the KISS principle in this case: do not interpret existing string. -- Duy -- 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: push.default documented in man git-push?
On Thu, Oct 11, 2012 at 9:18 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: For example, it makes the output of git status look like: # On branch master # Changes to be committed: # (use git reset HEAD file... to unstage) # Set advice.statusHints to false to turn off this message # # modified: foo.txt # # Changes not staged for commit: # (use git add file... to update what will be committed) # (use git checkout -- file... to discard changes in working directory) # Set advice.statusHints to false to turn off this message # # modified: foo.txt # # Untracked files: # (use git add file... to include in what will be committed) # Set advice.statusHints to false to turn off this message # # bar.txt And two more lines on my output: no changes added to commit (use git add and/or git commit -a) Set advice.statusHints to false to turn off this message I think it's going really too far in verbosity, the actual information is hidden by the advices. We could make it only one extra line in this case by prepending hint: before the advice and say you could turn the hints off by setting advice.statusHints at the end. Not applicable in general though. (which is good, more motivation to turn advice off). I disagree. Having advices turned on doesn't harm anyone. I don't remember anyone complaining about the verbosity of Git's advices. I've seen *many* more people complaining about the user-unfriendliness of Git. git status is actually quite verbose, but the advice only plays a part of it. So, not an actual complaint from me about the advice alone. - git checkout -b new_branch_name\n\n; + git checkout -b new_branch_name\n + Set either advice.detachedHead to false to turn off this message\n\n ^^ Wrong cut-and paste from ... either XXX or YYY ...? (repeated several times below). CutPaste mistake. -- Duy -- 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 3/3] grep: stop looking at random places for .gitattributes
On Wed, Oct 10, 2012 at 6:51 PM, Johannes Sixt j.s...@viscovery.net wrote: Thanks for working on this issue! Am 10/10/2012 13:34, schrieb Nguyễn Thái Ngọc Duy: + linkgit:gitattributes[5]). Note that .gitattributes are only + support for searching files in working directory. Does this mean it does not work for 'git grep --cached'? That would be a real loss. I missed this case. I don't think it works correctly before as it reads worktree's .gitattributes instead of the index version. But at least we support reading .gitattributes from index. Patches coming soon. -- Duy -- 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 3/3] grep: stop looking at random places for .gitattributes
On Wed, Oct 10, 2012 at 7:12 PM, Johannes Sixt j.s...@viscovery.net wrote: Is there already an established definition which the correct .gitattributes are? If I ask to grep the index then to me it should read only the index. Although other people can counter that they may want different attributes than the one stored in index, which either comes from worktree or $GIT_DIR. IIRC, everywhere else we are looking at the .gitattributes in the worktree, regardless of whether the object at the path in question is in the worktree, the index, or in an old commit. git-archive has --worktree-attributes to specify where attributes come from. Sparse checkout can choose to read index version first then worktree's or the other way around. All normal operations read worktree version, if not found index version. -- Duy -- 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 3/3] grep: stop looking at random places for .gitattributes
On Wed, Oct 10, 2012 at 7:43 PM, Johannes Sixt j.s...@viscovery.net wrote: Am 10/10/2012 14:32, schrieb Nguyen Thai Ngoc Duy: git-archive has --worktree-attributes to specify where attributes come from. Sparse checkout can choose to read index version first then worktree's or the other way around. All normal operations read worktree version, if not found index version. So, even if I run 'git diff master~2000 master~1999', it uses the attributes from the worktree, and if not found from the index? (I would not mind, BTW.) Yes. Tested and verified. -- Duy -- 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] attr: a note about the order of .gitattributes lookup
This is the documentation part of 1a9d7e9 (attr.c: read .gitattributes from index as well. - 2007-08-14) 06f33c1 (Read attributes from the index that is being checked out - 2009-03-13) Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- I looked around but did not see anywhere mentioning this. If I did not miss anything, then we should take a note about this to avoid surprises. Resend, this time git@vger is CCed. Sorry for the noise. Documentation/gitattributes.txt | 6 ++ 1 file changed, 6 insertions(+) diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt index 99ed04d..8c52a99 100644 --- a/Documentation/gitattributes.txt +++ b/Documentation/gitattributes.txt @@ -66,6 +66,12 @@ is from the path in question, the lower its precedence). Finally global and system-wide files are considered (they have the lowest precedence). +Normally if `.gitattributes` is not found in a directory in work tree, +the same path in the index is examined. If there's a `.gitattributes` +version in the index, that version will be used. During checkout process, +the order of examination is reversed: index version is preferred over +the work tree version. + If you wish to affect only a single repository (i.e., to assign attributes to files that are particular to one user's workflow for that repository), then -- 1.7.12.1.406.g6ab07c4 -- 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 2/2] attr: more matching optimizations from .gitignore
On Thu, Oct 11, 2012 at 4:41 AM, Junio C Hamano gits...@pobox.com wrote: Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: @@ -690,16 +689,18 @@ static int path_matches(const char *pathname, int pathlen, * contain the trailing slash */ - if (pathlen baselen || + if (pathlen baselen + 1 || (baselen pathname[baselen] != '/') || - strncmp(pathname, base, baselen)) + strncmp_icase(pathname, base, baselen)) Shouldn't the last comparison be strncmp_icase(pathname, base, baselen + 1) instead, base does not contain the trailing slash, so it can only match up to base[baselen-1], then fail at base[baselen], which is '\0'. The no trailing slash business in this function is tricky :( if you are trying to match this part from dir.c where baselen does count the trailing slash? if (pathlen x-baselen || (x-baselen pathname[x-baselen-1] != '/') || strncmp_icase(pathname, x-base, x-baselen)) continue; strncmp_icase() here just needs to compare x-baselen-1 chars (i.e. no trailing slash) as the trailing slash is explicitly checked just above strncmp_icase. But it does not hurt to compare an extra character so I leave it unchanged. But obviously it causes confusion when we try to match this function and the one in attr.c -- Duy -- 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 2/2] grep: stop looking at random places for .gitattributes
On Wed, Oct 10, 2012 at 9:21 PM, Johannes Sixt j.s...@viscovery.net wrote:
+ git commit -m new
+ echo Binary file HEAD:t matches expect
+ git grep text HEAD -- t actual
+ test_cmp expect actual
+ git reset HEAD^
+'
And in yet another test, should
git grep text HEAD:t
/not/ respect the binary attribute?
Gray area. Is it ok to do that without documenting it (i.e. common
sense)? I have something in mind that could do that, but it also makes
git grep text HEAD^{tree} not respect attributes.
--
Duy
--
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 needle rev' attempts to access 'rev:.../.gitattributes' in the worktree
On Tue, Oct 09, 2012 at 04:38:32PM +0700, Nguyen Thai Ngoc Duy wrote: #5 0x0815e736 in userdiff_find_by_path (path=0x820e7f0 HEAD:Documentation/.gitattributes) at userdiff.c:278 #6 0x081058ca in grep_source_load_driver (gs=0xbfffd978) at grep.c:1504 A bandage patch may look like this. But it does not solve the real problem: - we should be able to look up in-tree .gitattributes, not just ignore like this patch does - gs-name seems to be prepared for human consumption, not for accessing files. grep_file() with opt-relative is true can put quotes in gs-name, for example. I feel like we should make this function a callback and let git-grep deal with driver loading itself. -- 8 -- diff --git a/grep.c b/grep.c index edc7776..c5ed067 100644 --- a/grep.c +++ b/grep.c @@ -1501,7 +1501,8 @@ void grep_source_load_driver(struct grep_source *gs) return; grep_attr_lock(); - gs-driver = userdiff_find_by_path(gs-name); + if (gs-type == GREP_SOURCE_FILE) + gs-driver = userdiff_find_by_path(gs-name); if (!gs-driver) gs-driver = userdiff_find_by_name(default); grep_attr_unlock(); -- 8 -- -- Duy -- 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 4/8] wildmatch: remove static variable force_lower_case
On Wed, Oct 10, 2012 at 3:47 AM, Junio C Hamano gits...@pobox.com wrote: Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: diff --git a/wildmatch.c b/wildmatch.c index 7b64a6b..2382873 100644 --- a/wildmatch.c +++ b/wildmatch.c @@ -11,8 +11,8 @@ #include stddef.h #include ctype.h -#include string.h +#include cache.h #include wildmatch.h This is wrong; the includes from the system headers should have been removed in the previous step where the series integrated wildmatch to git, after which point the first include any C source that is not at the platform-compatibility layer should be cache.h or git-compat-util.h. Git's ctype does not seem to be complete for wildmatch's use so ctype.h is required. But that can be easily fixed later on. -- Duy -- 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 needle rev' attempts to access 'rev:.../.gitattributes' in the worktree
On Wed, Oct 10, 2012 at 1:59 AM, Junio C Hamano gits...@pobox.com wrote: Jeff King p...@peff.net writes: I think we just need to have callers of grep_source_init provide us with the actual pathname (or NULL, in the case of GREP_SOURCE_BUF). That is where the information is lost. Yes. I agree that is the right approach. Passing full path this way means prepare_attr_stack has to walk back to top directory for every files (even in the same directory). If .gitattributes are loaded while git-grep traverses the tree, then it can preload attr once per directory. But Jeff's approach looks simpler. -- Duy -- 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 8/8] Support ** wildcard in .gitignore and .gitattributes
On Tue, Oct 9, 2012 at 2:57 PM, Michael Haggerty mhag...@alum.mit.edu wrote: + - A trailing /** matches everything inside. For example, + abc/** is equivalent to `/abc/`. It seems odd that you add a leading slash in this example. I assume that is because of the rule that a pattern containing a slash is considered anchored at the current directory. But I find it confusing because the addition of the leading slash is not part of the rule you are trying to illustrate here, and is therefore a distraction. I suggest that you write either - A trailing /** matches everything inside. For example, /abc/** is equivalent to `/abc/`. or - A trailing /** matches everything inside. For example, abc/** is equivalent to `abc/` (which is also equivalent to `/abc/`). The tricky thing in .gitignore is that the last '/' alone does not imply anchor. So abc/ means match _directory_ abc anywhere in worktree. So the former is probably better. I should also add a note here (or in gitattributes.txt) about the difference between /abc/* and /abc/**. The former assigns attributes to all files directly under abc (e.g. depth 1), the latter infinite depth. + - A slash followed by two consecutive asterisks then a slash + matches zero or more directories. For example, `a/**/b` + matches `a/b`, `a/x/b`, `a/x/y/b` and so on. + + - Consecutive asterisks otherwise are treated like normal + asterisk wildcards. + I don't like the last rule. (1) This construct is superfluous; why wouldn't the user just use a single asterisk? (2) Allowing this construct means that it could appear in .gitignore files, creating unnecessary confusion: extrapolating from the other meanings of ** users would expect that it would somehow match slashes. (3) It is conceivable (though admittedly unlikely) that we might want to assign a distinct meaning to this construct in the future, and accepting it now as a different way to spell * would prevent such a change. Perhaps this rule was intended for backwards compatibility? We break backwards compatibility already. Existing **/ or /** patterns now behave differently. I think it would be preferable to say that other uses of consecutive asterisks are undefined, and probably make them trigger a warning. Instead of undefined, we can reject the pattern as broken. I have to check how fnmatch/wildmatch deals with broken patterns (it must do). If it returns a different code for broken patterns, then we can warn users, which is not limited in just ** breakage. -- Duy -- 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 needle rev' attempts to access 'rev:.../.gitattributes' in the worktree
On Wed, Oct 10, 2012 at 12:33 PM, Junio C Hamano gits...@pobox.com wrote: Nguyen Thai Ngoc Duy pclo...@gmail.com writes: On Wed, Oct 10, 2012 at 1:59 AM, Junio C Hamano gits...@pobox.com wrote: Jeff King p...@peff.net writes: I think we just need to have callers of grep_source_init provide us with the actual pathname (or NULL, in the case of GREP_SOURCE_BUF). That is where the information is lost. Yes. I agree that is the right approach. Passing full path this way means prepare_attr_stack has to walk back to top directory for every files (even in the same directory). If .gitattributes are loaded while git-grep traverses the tree, then it can preload attr once per directory. But Jeff's approach looks simpler. Why can't you do both? That is, to build a full path as you descend, and read per-directory .gitattributes as you go? Hm... I need to check attr.c code but I think it means read .gitattributes and prepare attr stack in builtin/grep.c (where we traverse trees) and actually check the attribute in grep_source_load_driver(), much further down the call stack. I'm not sure how we can pass the prepared attr stack down to grep_source_load_driver(). -- Duy -- 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 4/8] wildmatch: remove static variable force_lower_case
On Wed, Oct 10, 2012 at 12:31 PM, Junio C Hamano gits...@pobox.com wrote: Nguyen Thai Ngoc Duy pclo...@gmail.com writes: Git's ctype does not seem to be complete for wildmatch's use so ctype.h is required. But that can be easily fixed later on. Until later on, I cannot even compile the series. So that's why you noticed this patch :) It builds fine here. I'll fix up and send an update later. -- Duy -- 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 fetch documentation problem or bug
On Tue, Oct 9, 2012 at 6:33 AM, Junio C Hamano gits...@pobox.com wrote: * Unify pathspec semantics This has happened and commands that used to take only path prefix style pathspecs now take globs as well [ND] I've been thinking about it lately and will probably restart soon with a different approach. Still need to think about git-rm and git-mv as they also accept directories (i.e. not exactly pathspec by definition). -- Duy -- 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 04/10] attr: more matching optimizations from .gitignore
On Sat, Oct 6, 2012 at 1:48 AM, Junio C Hamano gits...@pobox.com wrote: Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: +Unlike `.gitignore`, negative patterns are not supported. +Patterns that match directories are also not supported. Is are not supported the right phrasing? I think it makes perfect sense not to forbid !path attr1, because it is unclear what it means (e.g. path -attr1 vs path !attr1). So I would say Negative patterns are forbidden as they do not make any sense. !path/sub/ alone does not mean anything. It must be used together with a positive pattern to define the set of paths the same attribute assignment statement applies to. This makes sense (attr, -attr1 or !attr1 are all OK): *.c attr1 !foo.c attr1 But this does not (actually !foo.c line has no effects because of different attribute assignments): *.c attr1 !foo.c attr2 It could be even more confusing in multiple attribute manipulation: *.c attr1 *.h -attr2 !foo.[ch] attr1 -attr2 So not supported and forbidden are equally OK. I just want to raise a point that it has some use before we go for forbidden. Nguyen Thai Ngoc Duy pclo...@gmail.com writes: On Sat, Oct 6, 2012 at 12:36 PM, Junio C Hamano gits...@pobox.com wrote: Or the user might think path/ attr1 sets attr1 for all files under path/ because it does not make sense to attach attributes to a directory in git. ... We may not have a need to assign a real attribute to a directory right now, because nothing in Git asks for an attribute for a directory. But that does not necessarily mean we would never need a way to give an attribute to a directory but not to its contents. Exactly why we should not make path/ attr no-op. If we want to make it meaningful some day in future, I don't think we want those no-ops lay around and suddenly cause changes in behavior with a new version of git. I do not think you understood. path/ attr is a no-op from the point of view of the *users* of the current versions of Git. It is perfectly fine to accept and apply attr to path/; no codepath in Git should be asking about path/ anyway, so it ends up to be a no-op. You shouldn't be erroring out at the syntactic level, i.e. when these lines are parsed. My objection is no-op lines are timed bombs. But users can already do dir attr (no slashes), which is no-op. So yeah, no-op is fine. -- Duy -- 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 04/10] attr: more matching optimizations from .gitignore
On Sat, Oct 6, 2012 at 12:36 PM, Junio C Hamano gits...@pobox.com wrote: Or the user might think path/ attr1 sets attr1 for all files under path/ because it does not make sense to attach attributes to a directory in git. ... We may not have a need to assign a real attribute to a directory right now, because nothing in Git asks for an attribute for a directory. But that does not necessarily mean we would never need a way to give an attribute to a directory but not to its contents. Exactly why we should not make path/ attr no-op. If we want to make it meaningful some day in future, I don't think we want those no-ops lay around and suddenly cause changes in behavior with a new version of git. If one does not think it through, the path/ excluded example might appear that there is no difference between setting exclude to the path itself and setting it to path and everything underneath it, but that comes largely from the nature of exclude attribute (think of exclude attribute as exclude itself and everything under it). From a user perspective, the thinking through portion is usually less than the try-and-see. There is no reason to assume other attributes we may want to give to a directory share the same recursive kind of semantics. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] attr: allow pattern escape using backslash
On Sat, Oct 6, 2012 at 2:54 PM, Junio C Hamano gits...@pobox.com wrote: Shouldn't we do the same for quoting fnmatch(3) metacharacters? To match a path component 'a' followed by an asterisk followed by 'b', you could then write 'a\*b'. Same for quoting the backslash itself. I think my patch does that too. The thing it does not do is optimize this case so that we can do strcmp() instead of fnmatch() if the entire pattern does not contain any metacharacters but backslashes. I don't think it'll become popular enough to deserve an optimization. -- Duy -- 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] gitignore.txt: suggestions how to get literal # or ! at the beginning
On Sat, Oct 6, 2012 at 6:33 PM, Philip Oakley philipoak...@iee.org wrote: Asciidoc 8.2.6 does not like me writing Put \# if you need a literal #.. so I go with backslash and hash instead. `\!` displays fine both in man page and html format. '!' changed to `!` because it looks clearer in monospace. Why not put the backslash-hash in back quotes as well to give the same look/feel consistency? Because asciidoc does not like \#, '\#' nor `\#`. It just shows \ without # and I don't want to master asciidoc just to make it show \#. -- Duy -- 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 am crash (builtin/apply.c:2108) + small repro
On Wed, Oct 3, 2012 at 10:44 PM, Alexey Spiridonov snarkmas...@gmail.com wrote: Thanks a lot for trying this. My hashes match. I just re-reproduced it on two flavors of Linux (64 and 32-bit), with two different Git versions (see below). What platform are you using? x86, 32 bit. Perhaps it has something to do with your configuration (config files, hooks, attributes). Assuming you use standard repository templates, you create new repo in your test so hooks and attributes are out. Is there anything suspicuous in git config -l? -- Duy -- 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: push.default documented in man git-push?
On Thu, Oct 4, 2012 at 12:45 PM, Junio C Hamano gits...@pobox.com wrote: Nguyen Thai Ngoc Duy pclo...@gmail.com writes: On Thu, Oct 4, 2012 at 1:49 AM, Junio C Hamano gits...@pobox.com wrote: I would recommend against listing any advice.* in the command manual pages. They are meant to give an advice in cases that are often confusing to new people and are supposed to advise how to turn it off in the message. OK. I think I was surprised that some messages were controlled by advice.* but gave no hints about that and I found that out by other means. I'll check all the advice messages. As far as I can tell, $ git grep -e 'advice\.' Documentation shows the list in config.txt and nothing else, and they do talk about when they are issued, but the reasoning behind them may not be described to a sufficient degree (that is, unless a reader carefully thinks things through, s/he may not be able to figure out why). But I think what we have there is more or less OK. It's about the commands, not the documents. For example I had no idea I could set advice.statusHints to turn off some advice. git status does not say that those messages can be turned off. -- Duy -- 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 05/10] Import wildmatch from rsync
On Fri, Oct 5, 2012 at 5:30 PM, Peter Krefting pe...@softwolves.pp.se wrote: These files are from rsync.git commit f92f5b166e3019db42bc7fe1aa2f1a9178cd215d, which was the last commit before rsync turned GPL-3. However: diff --git a/test-wildmatch.c b/test-wildmatch.c [...] + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. A copy mistake. I probably did not look closely at this file as it's not the main source, we could even rewrite it with little effort. Will update with GPL2 version next round. -- Duy -- 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 5/6] gitignore: do not do basename match with patterns that have '**'
On Fri, Oct 5, 2012 at 2:01 PM, Johannes Sixt j.s...@viscovery.net wrote: Am 10/4/2012 9:39, schrieb Nguyễn Thái Ngọc Duy: - - If the pattern does not contain a slash '/', git treats it as - a shell glob pattern and checks for a match against the - pathname relative to the location of the `.gitignore` file - (relative to the toplevel of the work tree if not from a - `.gitignore` file). + - If the pattern does not contain a slash '/' nor '**', git + treats it as a shell glob pattern and checks for a match + against the pathname relative to the location of the + `.gitignore` file (relative to the toplevel of the work tree + if not from a `.gitignore` file). I think in the latest round, we forbid this case (i.e. a/**, **/b and a/**/b are ok, but a**b is not), exactly because it's hard to define how it should do. Thanks for another example. +test_expect_success '** with no slashes test' ' + echo a**f foo=bar .gitattributes + cat \EOF expect +f: foo: unspecified +a/f: foo: bar +a/b/f: foo: bar +a/b/c/f: foo: bar +EOF Should the above .gitattributes match nested paths, such as b/a/c/f? I think it should, because the user can easily say /a**f that nested paths should not be matched. The user can also say **/a/**f to match b/a/c/f. -- Duy -- 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 (Oct 2012, #01; Tue, 2)
On Thu, Oct 04, 2012 at 09:39:02AM -0700, Junio C Hamano wrote:
Assuming that we do want to match x/y with x/**/y, I suspect
that '**' matches anything including a slash would not give us
that semantics. Is it something we can easily fix in the wildmatch
code?
Something like this may suffice. Lightly tested with git add -n.
Reading the code, I think we can even distinguish match zero or more
directories and match one or more directories with /**/ and maybe
/***/. Right now **, ***, ... are the same. So are /**/, /***/,
//...
-- 8 --
diff --git a/wildmatch.c b/wildmatch.c
index f153f8a..81eadc8 100644
--- a/wildmatch.c
+++ b/wildmatch.c
@@ -98,8 +98,12 @@ static int dowild(const uchar *p, const uchar *text,
continue;
case '*':
if (*++p == '*') {
+ int slashstarstar = p[-2] == '/';
while (*++p == '*') {}
special = TRUE;
+ if (slashstarstar *p == '/'
+ dowild(p + 1, text, a, force_lower_case) == TRUE)
+ return TRUE;
} else
special = FALSE;
if (*p == '\0') {
-- 8 --
--
Duy
--
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 04/10] attr: more matching optimizations from .gitignore
On Sat, Oct 6, 2012 at 1:48 AM, Junio C Hamano gits...@pobox.com wrote: Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: +Unlike `.gitignore`, negative patterns are not supported. +Patterns that match directories are also not supported. Is are not supported the right phrasing? I think it makes perfect sense not to forbid !path attr1, because it is unclear what it means (e.g. path -attr1 vs path !attr1). So I would say Negative patterns are forbidden as they do not make any sense. OK But for the latter, I think it makes a lot more sense to just accept path/ attr1 and doing nothing. The user requests to set an attribute to path that has to be a directory, and there is nothing wrong in such a request in itself. But nothing in git asks for attributes for directories (because we do not track directories), and such a request happens to be a no-op. Or the user might think path/ attr1 sets attr1 for all files under path/ because it does not make sense to attach attributes to a directory in git. -- Duy -- 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 (Oct 2012, #01; Tue, 2)
On Thu, Oct 4, 2012 at 4:34 PM, Michael Haggerty mhag...@alum.mit.edu wrote:
On Thu, Oct 4, 2012 at 4:34 PM, Michael Haggerty mhag...@alum.mit.edu wrote:
Given that there is no obvious interpretation for what a construct like
x**y would mean, and many plausible guesses (most of which sound
rather useless), I suggest that we forbid it. This will make the
feature easier to explain and make .gitignore files that use it easier
to understand.
Yep, sounds like a good short term plan.
As for the implementation, it is quite easy to textually convert a glob
pattern, including ** parts, into a regexp.
Or we could introduce regexp syntax as an alternative and let users
choose (and pay associated price). Patterns starting with // are never
matched (we don't normalize paths in .gitignore). Any patterns started
with //regex: is followed by regex. Reject all other // patterns for
future use.
_filename_char_pattern = r'[^/]'
_glob_patterns = [
('?', _filename_char_pattern),
('/**', r'(/.+)?'),
('**/', r'(.+/)?'),
('*', _filename_char_pattern + r'*'),
]
I don't fully understand the rest (never been a big fan of python) but
what about bracket expressions like [!abc] and [:alnum:]?
--
Duy
--
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: push.default documented in man git-push?
On Tue, Oct 2, 2012 at 10:09 PM, Ramkumar Ramachandra artag...@gmail.com wrote: David Glasser wrote: Is the newish push.default documented in the git push manpage anywhere? I don't see it mentioned (and there are several references to the default behavior), but maybe I'm missing something. Is it left out on purpose (ie, config values aren't supposed to be mentioned in command manpages)? You're right. It's documented in `man git-config`, but we should probably mention it in the `git-push` manpage. Your patch is fine. I'm just thinking whether it's a good idea to add a section in the end of each command's man page to list all relevant config keys to that command, somewhat similar to see also section. It may help finding useful config keys that otherwise hard to find. -- Duy -- 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
