* Re: Alternates corruption issue
From: Jeff King @ 2012-01-31 21:47 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Junio C Hamano, Richard Purdie, GIT Mailing-list, Hart, Darren,
Ashfield, Bruce
In-Reply-To: <20120131214047.GA13547@burratino>
On Tue, Jan 31, 2012 at 03:40:47PM -0600, Jonathan Nieder wrote:
> Jeff King wrote:
>
> > No, it does not match. While the sequence I outlined above makes the
> > most sense to me, it does not match what setup_git_directory does, which
> > prefers "foo/.git" to using "foo" as a bare repo. I think being
> > consistent between all of the lookup points makes sense. The patch took
> > the least-invasive approach and aligned clone and enter_repo with
> > setup_git_directory.
> >
> > However, we could also tweak setup_git_directory to prefer bare repos
> > over ".git" to keep things consistent. While it makes me feel good from
> > a theoretical standpoint (because the rules above seem simple and
> > intuitive to me), I'm not sure it's a good idea in practice.
>
> Wait, don't these two functions serve two completely different purposes?
Yes, but I would expect the lookup to be the same.
> One is the implementation of (A):
>
> cd foo
> git rev-parse --git-dir
>
> The other implements (B):
>
> git ls-remote foo
Right. But would you expect:
git ls-remote foo
to behave the same as:
cd foo
git ls-remote .
and would you furthermore expect that to behave the same as:
cd foo
git rev-parse --git-dir
?
Maybe I am crazy, but I see all of them as ways of specifying "I am
interested in repository foo", and any lookup magic should be the same
in all cases.
> If "foo" is actually a bare repository that moonlights as a worktree for
> a non-bare repository, then:
>
> 1) Whoever set up these directories is completely insane[*]. Maybe we
> should emit a warning.
Yes, I think we are dealing with an insane corner case.
> 2) As a naive user, I would expect (A) to give a different result
> from (B).
Why?
-Peff
^ permalink raw reply
* Re: Alternates corruption issue
From: Jonathan Nieder @ 2012-01-31 21:40 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Richard Purdie, GIT Mailing-list, Hart, Darren,
Ashfield, Bruce
In-Reply-To: <20120131204417.GA30969@sigill.intra.peff.net>
Jeff King wrote:
> No, it does not match. While the sequence I outlined above makes the
> most sense to me, it does not match what setup_git_directory does, which
> prefers "foo/.git" to using "foo" as a bare repo. I think being
> consistent between all of the lookup points makes sense. The patch took
> the least-invasive approach and aligned clone and enter_repo with
> setup_git_directory.
>
> However, we could also tweak setup_git_directory to prefer bare repos
> over ".git" to keep things consistent. While it makes me feel good from
> a theoretical standpoint (because the rules above seem simple and
> intuitive to me), I'm not sure it's a good idea in practice.
Wait, don't these two functions serve two completely different purposes?
One is the implementation of (A):
cd foo
git rev-parse --git-dir
The other implements (B):
git ls-remote foo
If "foo" is actually a bare repository that moonlights as a worktree for
a non-bare repository, then:
1) Whoever set up these directories is completely insane[*]. Maybe we
should emit a warning.
2) As a naive user, I would expect (A) to give a different result
from (B).
Hope that helps,
Jonathan
[*] ok, ok, they can be confused instead of insane:
http://bugs.debian.org/399041
^ permalink raw reply
* Re: [RFC/PATCH] add update to branch support for "floating submodules"
From: Jens Lehmann @ 2012-01-31 20:55 UTC (permalink / raw)
To: Phil Hord; +Cc: Junio C Hamano, Leif Gruenwoldt, git
In-Reply-To: <CABURp0pDoS1wgJ+Fs3XFX=A_EuR4Gzi4mHLiQP+-icT_d3J+WQ@mail.gmail.com>
Am 30.01.2012 22:15, schrieb Phil Hord:
> I lost my grip on this thread over the holidays...
>
> On Tue, Dec 13, 2011 at 4:09 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
>> Am 13.12.2011 15:17, schrieb Phil Hord:
>>> git pull origin topic_foo && git submodule foreach 'git pull origin topic_foo'
>>>
>>> git submodule foreach 'git push origin topic_foo' && git push origin topic_foo
>>
>> This sounds to me like you would need the "--recurse-submodules" option
>> implemented for "git pull" and "git push", no?
>
> Only if I have nested submodules, but yes, we do use --recurs* in our scripts.
I'm confused, push doesn't know the "--recurse-submodules" option
at all yet while pull only does a deep fetch when it is given, the
submodule work trees are not updated to the merge result right now.
>> And I miss to see how
>> floating would help when the tips of some submodules are not ready to
>> work with other submodules tips ...
>
> By project policy, for any branch, all submodules' tips of the
> same-named branch should be interoperable. The CI server looks after
> this, as much as he can.
We do the same thing on our CI server, but it can only test some
combinations (even though that tends to show most problems pretty
early). But in the end every superproject is responsible to use a
working set of submodule commits, and I would rather bet on a
combination the CI server tested than on what happens to be on the
current tips.
> I think of branch names as sticky notes (extra-lightweight tags,
> sometimes). We have linear history in many of our vendor submodules,
> but multiple "branches" indicate where each superproject branch has
> presumably finished integration.
We also add a branch in submodules every time a superproject needs
to move away from the submodules master (so the commits won't get
lost by accident).
>>> But not all my developers are git-gurus yet, and they sometimes mess
>>> up their ad hoc scripts or miss important changes they forgot to push
>>> in one submodule or another.
>>
>> Sure, even though current git should help you some by showing changes
>> in the submodules.
>
> Real newbies may not even remember to use 'git status' strategically.
Hmm, but then they will screw up things in the superproject too, no?
>>> Or worse, their pull or push fails and
>>> they can't see the problem for all the noise. So they email it to me.
>>
>> We circumvent that by not pulling, but fetching and merging in the
>> submodule first and after that in the superproject. You have much more
>> control about what is going wrong where (and can have more
>> git-experienced people help with - or even do - the merges).
>
> I do that, too, and I wish I didn't have to. I wish I could safely
> and sanely recover from a conflicted "git pull --recurse-submodules"
> pull from the superproject.
That's what my recursive checkout work is aiming at. Me thinks after
that we will also need some good ideas on how to present and help
solving submodule merge conflicts.
> That is, I wish doing so were as
> straightforward as recovering from the same condition would be if all
> my code were in one repository instead of in submodules.
>
> Which is the gist -- I wish submodules did not make git more
> complicated than it already is.
I think we can make working with submodule much easier than it is
now, the next step being updating all submodule work trees as git
updates the superproject's work tree. Even though I suspect that in
the long run submodules will always be a bit more complicated than
having everything in one repository, I'm confident that will be by
far outweighed by the advantages they bring.
^ permalink raw reply
* Re: Alternates corruption issue
From: Jeff King @ 2012-01-31 20:44 UTC (permalink / raw)
To: Junio C Hamano
Cc: Richard Purdie, GIT Mailing-list, Hart, Darren, Ashfield, Bruce
In-Reply-To: <7v1uqf8vqu.fsf@alter.siamese.dyndns.org>
On Tue, Jan 31, 2012 at 12:25:45PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > I believe that would work in your case, but it seems like the most
> > correct thing would actually be:
> >
> > { "", "/.git", ".git" }
> >
> > That is:
> >
> > 1. Try the literal path the user gave as a repo
> >
> > 2. Otherwise, try it as the root of a working tree (containing .git)
> >
> > 3. Otherwise, assume they were too lazy to type ".git" and include it
>
> That sounds sensible, together with this, to which I agree with:
> [...]
> ... but ...
> [...]
> > - static char *suffix[] = { "/.git", ".git", "" };
> > + static char *suffix[] = { "/.git", "", ".git" };
>
> ... this does not match that simple and clear guideline.
>
> Shouldn't this simply be { "", "/.git", ".git" }?
No, it does not match. While the sequence I outlined above makes the
most sense to me, it does not match what setup_git_directory does, which
prefers "foo/.git" to using "foo" as a bare repo. I think being
consistent between all of the lookup points makes sense. The patch took
the least-invasive approach and aligned clone and enter_repo with
setup_git_directory.
However, we could also tweak setup_git_directory to prefer bare repos
over ".git" to keep things consistent. While it makes me feel good from
a theoretical standpoint (because the rules above seem simple and
intuitive to me), I'm not sure it's a good idea in practice.
The case we would "catch" with such a change is when you have a ".git"
directory inside a bare repo. Right now, we prefer the ".git" inside it,
and ignore the containing bare repo. With such a change, we would prefer
the outer bare repo. Which makes sense to me. It does break a use
case like this:
# make a new repo
git init
cd .git
# now track parts of the repo
git init
git add config
git commit -m 'tracking our repo config'
but I'm not sure if that is sane or not.
But also consider false positives. What if you have a repository that
looks like a git repo (i.e., has "objects", "refs", and "HEAD" in it),
but also has ".git". Right now we say "Oh, it has .git, that must be the
repo". But with the proposed change, we could accidentally find the
enclosing repo.
Now, the chances of is_git_directory being wrong seem quite slim to me.
But then, the chances of somebody actually have a repo with a ".git"
_inside_ it seem pretty slim to me. So I think we are really dealing
with a tiny corner case, and it is perhaps anybody's guess whether
anyone is depending on the current behavior in the wild. I don't overly
care either way, but when in doubt, I tend to stick with the existing
behavior.
> > if (!strict) {
> > static const char *suffix[] = {
> > - ".git/.git", "/.git", ".git", "", NULL,
> > + "/.git", "", ".git/.git", ".git", NULL,
> > };
>
> Neither does this.
>
> Shouldn't this be { "", "/.git", ".git", ".git/.git", NULL }?
Right, same case.
> I must be missing something from your description...
I mentioned the issue in my original message, but perhaps didn't
emphasize it very well.
-Peff
^ permalink raw reply
* Re: Alternates corruption issue
From: Junio C Hamano @ 2012-01-31 20:25 UTC (permalink / raw)
To: Jeff King; +Cc: Richard Purdie, GIT Mailing-list, Hart, Darren, Ashfield, Bruce
In-Reply-To: <20120131193922.GA31551@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I believe that would work in your case, but it seems like the most
> correct thing would actually be:
>
> { "", "/.git", ".git" }
>
> That is:
>
> 1. Try the literal path the user gave as a repo
>
> 2. Otherwise, try it as the root of a working tree (containing .git)
>
> 3. Otherwise, assume they were too lazy to type ".git" and include it
That sounds sensible, together with this, to which I agree with:
> One way of dealing with that would be to make get_repo_path a little
> more robust by only selecting paths which actually look like git
> directories.
... but ...
> diff --git a/builtin/clone.c b/builtin/clone.c
> index 9084feb..0fbbae9 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -108,7 +108,7 @@ static const char *argv_submodule[] = {
>
> static char *get_repo_path(const char *repo, int *is_bundle)
> {
> - static char *suffix[] = { "/.git", ".git", "" };
> + static char *suffix[] = { "/.git", "", ".git" };
... this does not match that simple and clear guideline.
Shouldn't this simply be { "", "/.git", ".git" }?
> diff --git a/path.c b/path.c
> index b6f71d1..1ca6567 100644
> --- a/path.c
> +++ b/path.c
> @@ -293,7 +293,7 @@ const char *enter_repo(const char *path, int strict)
>
> if (!strict) {
> static const char *suffix[] = {
> - ".git/.git", "/.git", ".git", "", NULL,
> + "/.git", "", ".git/.git", ".git", NULL,
> };
Neither does this.
Shouldn't this be { "", "/.git", ".git", ".git/.git", NULL }?
I must be missing something from your description...
^ permalink raw reply
* Re: [PATCH] vcs-svn: Fix some compiler warnings
From: Junio C Hamano @ 2012-01-31 20:14 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Ramsay Jones, David Barr, GIT Mailing-list
In-Reply-To: <20120131192053.GC12443@burratino>
Jonathan Nieder <jrnieder@gmail.com> writes:
> off_t delta_len = off_t_or_die(len, "enormous delta");
> postimage_len = apply_delta(delta_len, input, ...);
>
> What do you think?
Another possibility would be to make the "die" part responsibility of the
caller of the helper function, e.g.
if (value_out_of_range(off_t, len))
die("enormous delta");
which may make the caller easier to follow and the helper easier to reuse.
^ permalink raw reply
* Re: [PATCH] Fix an "variable might be used uninitialized" gcc warning
From: Jonathan Nieder @ 2012-01-31 19:43 UTC (permalink / raw)
To: Ramsay Jones; +Cc: Junio C Hamano, GIT Mailing-list
In-Reply-To: <4F2834AD.20004@ramsay1.demon.co.uk>
Ramsay Jones wrote:
> The versions which complain are 3.4.4 and 4.1.2, whereas 4.4.0 compiles
> the code without complaint. So, gcc *may* be getting more sane, but I wouldn't
> bet on it! :-P
>
> I've had examples of this kind of warning, which relies heavily on the
> analysis performed primarily for the optimizer, come-and-go in gcc before
Yep, judging from the commit message, Junio found the same warning
in 4.6.2.
[...]
> Having said that, unless you are going to decree that the project only
> supports gcc (and presumably only some particular versions of gcc), then you
> may well find similar warnings triggered when using other compilers anyway ...
Sure, when the control flow grows too complicated, that's probably worth
fixing anyway, for the sake of humans especially.
Sometimes gcc is the only crazy one, though. ;-)
Thanks for the update.
Jonathan
^ permalink raw reply
* Re: Alternates corruption issue
From: Jeff King @ 2012-01-31 19:39 UTC (permalink / raw)
To: Richard Purdie; +Cc: GIT Mailing-list, Hart, Darren, Ashfield, Bruce
In-Reply-To: <1328018729.13744.26.camel@ted>
On Tue, Jan 31, 2012 at 02:05:29PM +0000, Richard Purdie wrote:
> I have a problem with git clone commands using alternates failing by
> mixing up different repositories. I have a situation where I could end
> up with both:
>
> /srv/mirrors/repo
> /srv/mirrors/repo.git
I think the alternates thing is just a side effect. The real problem is
that clone is picking the wrong repo to clone from! So a simple complete
test would be something like:
# make confusing bare repos
git init --bare bare &&
git init --bare bare.git &&
# now make some commits; after this
# segment, "bare" should have its tip
# at commit "one", and "bare.git" should
# be at "two".
git init source &&
(cd source &&
echo content >file &&
git add file &&
git commit -m one &&
git push ../bare HEAD &&
echo content >>file &&
git commit -a -m two &&
git push ../bare.git HEAD
) &&
# now try to clone bare and see which one we get;
# we would expect the tip to be at "one", but it's not
git clone bare clone &&
cd clone &&
echo one >expect &&
git log -1 --format=%s >actual &&
diff -u expect actual
# we cloned the wrong repository!
> Looking at the code, the cause seems to be
>
> clone.c:get_repo_path():
>
> static char *suffix[] = { "/.git", ".git", "" };
>
> since its looking in order for:
> repo/.git (fails)
> repo.git (suceeds, incorrect)
> repo (never looked at)
Yeah, this seems brain-dead. It's good that git looks through a magic
list to guess what I meant, but shouldn't the first thing it tries be
the literal string that I gave, and only _then_ kick in the magic?
But even weirder, this doesn't match the lookup in other parts of git. I
see four distinct places in git where we do repository lookup:
1. get_repo_path used by clone (i.e., this)
2. enter_repo used by upload-pack, receive-pack, and git-daemon; this
looks first for ".git/.git", to find a non-bare repo which is named
like a bare one.
3. setup_git_directory_*, used by most git programs. This starts
looking in the current directory and going up, looking first for
".git", and then checking for a bare repo. So if you were to do
"cd bare && git foo", then we would correctly find the current
directory, not "bare.git".
4. setup_explicit_git_dir, which gets called if you use --git-dir or
set GIT_DIR. This expects to find the repo directly in the
directory you specified, with no magic.
So I think (3) and (4) are sensible, but I tend to think that (1) and
(2) are wrong, and should prefer the directory you gave before the
magic.
> I'm not sure what would break if that order were to change, swapping the
> last two options.
I believe that would work in your case, but it seems like the most
correct thing would actually be:
{ "", "/.git", ".git" }
That is:
1. Try the literal path the user gave as a repo
2. Otherwise, try it as the root of a working tree (containing .git)
3. Otherwise, assume they were too lazy to type ".git" and include it
If you simply swap the last two, then there is one obscure case you
would miss: a bare repo which contains a .git directory. Of course, that
seems like an insane thing to have, but if you did have it, I would
expect git to follow the rules I outlined above. That being said,
if you follow the rules in setup_git_directory_gently_1, it actually
prefers "/.git" to a bare repo. So probably your swap is better, as it
matches the lookup here with the regular setup_git_directory one.
One slight complication is that get_repo_path only checks whether the
paths exist, not whether they are actually git repositories. So by
simply swapping the order, you are breaking some other obscure cases,
like where a "foo" directory exists but is _not_ a git repo, and a
"foo.git" exists alongside it (which sounds a bit crazy, but I could see
somebody using that scheme if they wanted didn't want the git repository
to live inside the worktree for some reason).
One way of dealing with that would be to make get_repo_path a little
more robust by only selecting paths which actually look like git
directories.
The patch below does this, and makes my test above pass. But I think
before talking too much about code, it would be best to decide on what
the lookup order _should_ be, and then we can write tests and implement
to them.
-Peff
---
diff --git a/builtin/clone.c b/builtin/clone.c
index 9084feb..0fbbae9 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -108,7 +108,7 @@ static const char *argv_submodule[] = {
static char *get_repo_path(const char *repo, int *is_bundle)
{
- static char *suffix[] = { "/.git", ".git", "" };
+ static char *suffix[] = { "/.git", "", ".git" };
static char *bundle_suffix[] = { ".bundle", "" };
struct stat st;
int i;
@@ -118,7 +118,7 @@ static char *get_repo_path(const char *repo, int *is_bundle)
path = mkpath("%s%s", repo, suffix[i]);
if (stat(path, &st))
continue;
- if (S_ISDIR(st.st_mode)) {
+ if (S_ISDIR(st.st_mode) && is_git_directory(path)) {
*is_bundle = 0;
return xstrdup(absolute_path(path));
} else if (S_ISREG(st.st_mode) && st.st_size > 8) {
diff --git a/cache.h b/cache.h
index 9bd8c2d..7b2857f 100644
--- a/cache.h
+++ b/cache.h
@@ -436,6 +436,7 @@ extern char *get_object_directory(void);
extern char *get_index_file(void);
extern char *get_graft_file(void);
extern int set_git_dir(const char *path);
+extern int is_git_directory(const char *path);
extern const char *get_git_namespace(void);
extern const char *strip_namespace(const char *namespaced_ref);
extern const char *get_git_work_tree(void);
diff --git a/path.c b/path.c
index b6f71d1..1ca6567 100644
--- a/path.c
+++ b/path.c
@@ -293,7 +293,7 @@ const char *enter_repo(const char *path, int strict)
if (!strict) {
static const char *suffix[] = {
- ".git/.git", "/.git", ".git", "", NULL,
+ "/.git", "", ".git/.git", ".git", NULL,
};
const char *gitfile;
int len = strlen(path);
@@ -324,8 +324,11 @@ const char *enter_repo(const char *path, int strict)
return NULL;
len = strlen(used_path);
for (i = 0; suffix[i]; i++) {
+ struct stat st;
strcpy(used_path + len, suffix[i]);
- if (!access(used_path, F_OK)) {
+ if (!stat(used_path, &st) &&
+ (S_ISREG(st.st_mode) ||
+ (S_ISDIR(st.st_mode) && is_git_directory(used_path)))) {
strcat(validated_path, suffix[i]);
break;
}
diff --git a/setup.c b/setup.c
index 61c22e6..7a3618f 100644
--- a/setup.c
+++ b/setup.c
@@ -247,7 +247,7 @@ const char **get_pathspec(const char *prefix, const char **pathspec)
* a proper "ref:", or a regular file HEAD that has a properly
* formatted sha1 object name.
*/
-static int is_git_directory(const char *suspect)
+int is_git_directory(const char *suspect)
{
char path[PATH_MAX];
size_t len = strlen(suspect);
^ permalink raw reply related
* Re: [PATCH v2] find_pack_entry(): do not keep packed_git pointer locally
From: Nicolas Pitre @ 2012-01-31 19:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nguyễn Thái Ngọc Duy, git
In-Reply-To: <7vr4yf92dg.fsf@alter.siamese.dyndns.org>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1509 bytes --]
On Tue, 31 Jan 2012, Junio C Hamano wrote:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
> > + /*
> > + * p can be NULL from the else clause above, if initial
> > + * f_p_e_last_found value (i.e. INVALID_PACK) is NULL, we may
> > + * advance p again to an imaginary pack in invalid memory
> > + */
Ah! OK so that's why I initially came up with that (void*)1 value.
> But I think the real issue is that the original loop is written in an
> obscure way.
Can't disagree with that, especially when the original author (myself)
doesn't see clearly through it anymore.
> The conversion in f7c22cc (always start looking up objects in the last
> used pack first, 2007-05-30) wanted to turn the traversal that always
> went from the tip of a linked list to instead first probe the
> promising one, and then scan the list from the tip like it used to do,
> except that it did not want to probe the one it thought promising
> again.
Exact.
> So perhaps restructuring the loop by making the logic to probe into a
> single pack into a helper function, e.g.
>
> static int find_pack_entry(const unsigned char *sha1, struct pack_entry *e)
> {
> if (last_found) {
> if (find_one(sha1, e, last_found))
> return 1;
> }
> for (p = packed_git; p; p = p->next) {
> if (p == last_found || !find_one(sha1, e, p))
> continue;
> last_found = p;
> return 1;
> }
> return 0;
> }
>
> would make the resulting flow far easier to follow, no?
Indeed.
Nicolas
^ permalink raw reply
* Re: [PATCH] vcs-svn: Fix some compiler warnings
From: Jonathan Nieder @ 2012-01-31 19:20 UTC (permalink / raw)
To: Ramsay Jones; +Cc: David Barr, Junio C Hamano, GIT Mailing-list
In-Reply-To: <4F28378F.6080108@ramsay1.demon.co.uk>
Hi,
Ramsay Jones wrote:
> In particular, some versions of gcc complains as follows:
>
> CC vcs-svn/sliding_window.o
> vcs-svn/sliding_window.c: In function `check_overflow':
> vcs-svn/sliding_window.c:36: warning: comparison is always false \
> due to limited range of data type
Yuck. Suppressing this warning would presumably also suppress the
optimization that notices the comparison is always false.
The -Wtype-limits warning also triggers in some other perfectly
reasonable situations: see <http://gcc.gnu.org/PR51712>. I wonder if
we should keep a list of unreliable warnings somewhere (e.g.,
Meta/Make).
[...]
> Note that the "some versions of gcc" which complain includes 3.4.4 and
> 4.1.2, whereas gcc version 4.4.0 compiles the code without complaint.
Thanks for tracking this down. Interesting. -Wtype-limits was split
out from the default set of warnings (!) in gcc 4.3 to address
<http://gcc.gnu.org/PR12963>, among other bugs (r124875, 2007-05-20).
[...]
> --- a/vcs-svn/fast_export.c
> +++ b/vcs-svn/fast_export.c
> @@ -300,7 +300,8 @@ void fast_export_blob_delta(uint32_t mode,
> uint32_t len, struct line_buffer *input)
> {
> long postimage_len;
> - if (len > maximum_signed_value_of_type(off_t))
> + uintmax_t delta_len = (uintmax_t) len;
> + if (delta_len > maximum_signed_value_of_type(off_t))
> die("enormous delta");
> postimage_len = apply_delta((off_t) len, input, old_data, old_mode);
Is there some less ugly way to write the condition "if this value is
not representable in this type"?
I guess I could live with something like the following (please don't
take the names too seriously):
static inline off_t off_t_or_die(uintmax_t val, const char *msg_if_bad)
{
if (val > maximum_signed_value_of_type(off_t))
die("%s", msg_if_bad);
return (off_t) val;
}
...
off_t delta_len = off_t_or_die(len, "enormous delta");
postimage_len = apply_delta(delta_len, input, ...);
What do you think?
Jonathan
^ permalink raw reply
* [PATCH] vcs-svn: Fix some compiler warnings
From: Ramsay Jones @ 2012-01-31 18:48 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: david.barr, Junio C Hamano, GIT Mailing-list
In particular, some versions of gcc complains as follows:
CC vcs-svn/sliding_window.o
vcs-svn/sliding_window.c: In function `check_overflow':
vcs-svn/sliding_window.c:36: warning: comparison is always false \
due to limited range of data type
CC vcs-svn/fast_export.o
vcs-svn/fast_export.c: In function `fast_export_blob_delta':
vcs-svn/fast_export.c:303: warning: comparison is always false due \
to limited range of data type
Simply casting the (limited range unsigned) variable in the comparison
to an uintmax_t does not suppress the warning, however, since gcc is
"smart" enough to know that the cast does not change anything regarding
the value of the casted expression. In order to suppress the warning, we
replace the variable with a new uintmax_t variable initialized with the
value of the original variable.
Note that the "some versions of gcc" which complain includes 3.4.4 and
4.1.2, whereas gcc version 4.4.0 compiles the code without complaint.
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
Hi Jonathan,
When I wrote this patch, your "jn/svn-fe" branch was still in pu, so I was
going to ask you to squash this, or some variant, into any re-roll ...
Note, in the original version of this patch, the change to check_overflow()
was much simpler; it was effectively the same 2-line change (modulo some
variable names) which is applied to fast_export_blob_delta(). But it just
didn't look right (not sure why!), which prompted the renaming of the
function parameter names. This is, obviously, an unrelated change, so maybe
the change to sliding_window.c should be reverted to the 2-line change ...
ATB,
Ramsay Jones
vcs-svn/fast_export.c | 3 ++-
vcs-svn/sliding_window.c | 11 ++++++-----
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c
index 19d7c34..6edd37e 100644
--- a/vcs-svn/fast_export.c
+++ b/vcs-svn/fast_export.c
@@ -300,7 +300,8 @@ void fast_export_blob_delta(uint32_t mode,
uint32_t len, struct line_buffer *input)
{
long postimage_len;
- if (len > maximum_signed_value_of_type(off_t))
+ uintmax_t delta_len = (uintmax_t) len;
+ if (delta_len > maximum_signed_value_of_type(off_t))
die("enormous delta");
postimage_len = apply_delta((off_t) len, input, old_data, old_mode);
if (mode == REPO_MODE_LNK) {
diff --git a/vcs-svn/sliding_window.c b/vcs-svn/sliding_window.c
index 1bac7a4..49a7293 100644
--- a/vcs-svn/sliding_window.c
+++ b/vcs-svn/sliding_window.c
@@ -31,15 +31,16 @@ static int read_to_fill_or_whine(struct line_buffer *file,
return 0;
}
-static int check_overflow(off_t a, size_t b)
+static int check_overflow(off_t offset, size_t len)
{
- if (b > maximum_signed_value_of_type(off_t))
+ uintmax_t delta_len = (uintmax_t) len;
+ if (delta_len > maximum_signed_value_of_type(off_t))
return error("unrepresentable length in delta: "
- "%"PRIuMAX" > OFF_MAX", (uintmax_t) b);
- if (signed_add_overflows(a, (off_t) b))
+ "%"PRIuMAX" > OFF_MAX", delta_len);
+ if (signed_add_overflows(offset, (off_t) len))
return error("unrepresentable offset in delta: "
"%"PRIuMAX" + %"PRIuMAX" > OFF_MAX",
- (uintmax_t) a, (uintmax_t) b);
+ (uintmax_t) offset, delta_len);
return 0;
}
--
1.7.9
^ permalink raw reply related
* Re: [PATCH] Fix an "variable might be used uninitialized" gcc warning
From: Ramsay Jones @ 2012-01-31 18:36 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, GIT Mailing-list
In-Reply-To: <20111216235908.GA5858@elie.hsd1.il.comcast.net>
Jonathan Nieder wrote:
> Ramsay Jones wrote:
>
>> CC builtin/checkout.o
>> builtin/checkout.c: In function `cmd_checkout':
>> builtin/checkout.c:160: warning: 'mode' might be used uninitialized \
>> in this function
> [...]
>> [Note that only 2 out of the 3 versions of gcc I use issues this
>> warning]
>
> Which version of gcc is that? Is gcc getting more sane, so we won't
> have to worry about this after a while, or is the false positive a
> new regression that should be reported to them?
[Sorry for the late reply, I've been away from email for several weeks...]
The versions which complain are 3.4.4 and 4.1.2, whereas 4.4.0 compiles
the code without complaint. So, gcc *may* be getting more sane, but I wouldn't
bet on it! :-P
I've had examples of this kind of warning, which relies heavily on the
analysis performed primarily for the optimizer, come-and-go in gcc before; so
don't hold your breath (this is the most volatile part of the compiler).
Having said that, unless you are going to decree that the project only
supports gcc (and presumably only some particular versions of gcc), then you
may well find similar warnings triggered when using other compilers anyway ...
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [PATCH v2] find_pack_entry(): do not keep packed_git pointer locally
From: Junio C Hamano @ 2012-01-31 18:02 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, nico
In-Reply-To: <1328010239-29669-1-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> Changing INVALID_PACK to NULL breaks at least t1050.4. We may
> restructure the p assignments at the end of find_pack_entry() to
> allow setting INVALID_PACK to NULL.
>
> But I'm not really excited about that.
The patch to do so should be a trivial one on top of this anyway
(attached).
> - if (p == last_found)
> + if (p == find_pack_entry_last_found)
> p = packed_git;
> else
> p = p->next;
> - if (p == last_found)
> + /*
> + * p can be NULL from the else clause above, if initial
> + * f_p_e_last_found value (i.e. INVALID_PACK) is NULL, we may
> + * advance p again to an imaginary pack in invalid memory
> + */
> + if (p == find_pack_entry_last_found)
> p = p->next;
But I think the real issue is that the original loop is written in an
obscure way. The conversion in f7c22cc (always start looking up objects
in the last used pack first, 2007-05-30) wanted to turn the traversal that
always went from the tip of a linked list to instead first probe the
promising one, and then scan the list from the tip like it used to do,
except that it did not want to probe the one it thought promising again.
So perhaps restructuring the loop by making the logic to probe into a
single pack into a helper function, e.g.
static int find_pack_entry(const unsigned char *sha1, struct pack_entry *e)
{
if (last_found) {
if (find_one(sha1, e, last_found))
return 1;
}
for (p = packed_git; p; p = p->next) {
if (p == last_found || !find_one(sha1, e, p))
continue;
last_found = p;
return 1;
}
return 0;
}
would make the resulting flow far easier to follow, no?
sha1_file.c | 16 +++++-----------
1 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 77512a3..2286789 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -54,9 +54,7 @@ static struct cached_object empty_tree = {
0
};
-/* INVALID_PACK cannot be NULL, see comments in find_pack_entry */
-#define INVALID_PACK (struct packed_git *)1
-static struct packed_git *find_pack_entry_last_found = INVALID_PACK;
+static struct packed_git *find_pack_entry_last_found;
static struct cached_object *find_cached_object(const unsigned char *sha1)
{
@@ -725,7 +723,7 @@ void free_pack_by_name(const char *pack_name)
free(p->bad_object_sha1);
*pp = p->next;
if (find_pack_entry_last_found == p)
- find_pack_entry_last_found = INVALID_PACK;
+ find_pack_entry_last_found = NULL;
free(p);
return;
}
@@ -2024,7 +2022,7 @@ static int find_pack_entry(const unsigned char *sha1, struct pack_entry *e)
prepare_packed_git();
if (!packed_git)
return 0;
- p = find_pack_entry_last_found == INVALID_PACK ? packed_git : find_pack_entry_last_found;
+ p = !find_pack_entry_last_found ? packed_git : find_pack_entry_last_found;
do {
if (p->num_bad_objects) {
@@ -2060,12 +2058,8 @@ static int find_pack_entry(const unsigned char *sha1, struct pack_entry *e)
p = packed_git;
else
p = p->next;
- /*
- * p can be NULL from the else clause above, if initial
- * f_p_e_last_found value (i.e. INVALID_PACK) is NULL, we may
- * advance p again to an imaginary pack in invalid memory
- */
- if (p == find_pack_entry_last_found)
+
+ if (p && p == find_pack_entry_last_found)
p = p->next;
} while (p);
return 0;
^ permalink raw reply related
* Re: [PATCH] Correct singular form in diff summary line for human interaction
From: Junio C Hamano @ 2012-01-31 17:50 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Nguyễn Thái Ngọc Duy, git,
Ævar Arnfjörð Bjarmason, Frederik Schwarzer,
Brandon Casey, dickey
In-Reply-To: <20120131152028.GA10717@burratino>
Jonathan Nieder <jrnieder@gmail.com> writes:
> Nguyễn Thái Ngọc Duy wrote:
>
>> Convenient function interactive_use() is added for this purpose. The
>> command thinks it's in human hands when:
>
> I admit I really dislike this, especially:
>
>> - GIT_SCRIPTING environment variable is not set
I would have to agree that it is horrible.
> But maybe I'm not the right person to ask, since I'd be okay with
> removing the "s"es (with an appropriate incubation time to discover
> whether we are introducing a regression) unconditionally.
>
> If there is an environment variable to say "I don't want to see
> variations on strings intended for humans", can it be spelled as
> LC_ALL=C?
I have been wondering if we should even care, for two reasons.
* We have had --numstat forever which is exactly what we added for scripts'
use. "I've been parsing that output meant for humans" is not an excuse.
* 'diffstat', at least the recent versions of it (it is hard to track
down historical versions and I gave up [*1*]), gives output like these:
1 file changed, 1 insertion(+)
2 files changed, 3 insertions(+), 1 deletion(-)
0 files changed
The first one does not have anything but a one-line addition to a file,
and we do not even see "0 deletions(-)". The second one is a more
typical example. The third one is "diffstat </dev/null" [*2*].
If we were to touch this, I would prefer to do so unconditionally without
"hrm, can we reliably guess this is meant for humans?" and release it
unceremoniously, perhaps as part of the next release that will have a much
bigger user-visible UI correction to 'merge'.
[Footnote]
*1* I can guess from http://invisible-island.net/diffstat/CHANGES that the
source is probably kept in RCS, but I couldn't find development histories
beyond what is in that file.
*2* We mistakenly applied a patch to make "git apply --stat </dev/null" to
error out recently, which we might want to fix. But that is a separate
topic.
^ permalink raw reply
* Re: [BUG] git clean -X skips a directory containing only ignored files
From: Paul Berry @ 2012-01-31 17:39 UTC (permalink / raw)
To: Michael Schubert; +Cc: git
In-Reply-To: <4F27FF01.6040706@elegosoft.com>
On 31 January 2012 06:47, Michael Schubert <mschub@elegosoft.com> wrote:
> On 01/31/2012 12:36 AM, Paul Berry wrote:
>> I am trying to use "git clean -X" to remove object files (which
>> are gitignored) from my source tree, but it appears to miss
>> object files that are in a subdirectory without any git-tracked
>> files:
>>
>> $ git init test
>> Initialized empty Git repository in /home/pberry/tmp/test/.git/
>> $ cd test
>> $ mkdir foo
>> $ touch foo/bar.o
>> $ echo '*.o' > .gitignore
>> $ git add .gitignore
>> $ git commit -mgitignore
>> [master (root-commit) 6b5ffcb] gitignore
>> 1 files changed, 1 insertions(+), 0 deletions(-)
>> create mode 100644 .gitignore
>> $ git status
>> # On branch master
>> nothing to commit (working directory clean)
>> $ git clean -d -X -f
>> $ ls foo
>> bar.o
>>
>> It seems to me that bar.o should have been removed, because
>> according to the git-clean docs, -X means "Remove only files
>> ignored by git", and bar.o is definitely being ignored by git.
>>
>>
>> It looks like a very similar bug was reported back in 2010, but
>> not fixed:
>> http://git.661346.n2.nabble.com/BUG-git-clean-X-behaviour-when-gitignore-has-sub-directory-entries-td5575307.html.
>> I've confirmed that the workaround mentioned by Jonathan Nieder
>> in that thread fixes my problem too (removing "dir.flags |=
>> DIR_SHOW_OTHER_DIRECTORIES;" from builtin/clean.c). However I'm
>> guessing from Jonathan's comments that it would be better to fix
>> this bug elsewhere (somewhere in dir.c perhaps).
>
> Removing DIR_SHOW_OTHER_DIRECTORIES just happens to not trigger
> this particular "bug" but breaks pretty much everything else.
Yeah, I had a feeling that might be the case.
>
> As a workaround, you could explicitly add the directory to your
> gitignore file.
>
> Here's a test:
>
> -- >8 --
>
> Subject: [PATCH] t7300-clean: show known breakage with "git clean -d -X"
>
> "git clean -d -X" fails for directories containing only untracked files.
> Example:
>
> $ ls -R .
> .:
> foo
> ./foo:
> bar.o
> $ cat .gitignore
> *.o
> $ git clean -d -X -f
> $ ! test -d foo || echo fail
>
> Reported-by: Paul Berry <stereotype441@gmail.com>
> Signed-off-by: Michael Schubert <mschub@elegosoft.com>
> ---
> t/t7300-clean.sh | 7 +++++++
> 1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
> index 800b536..0b6d545 100755
> --- a/t/t7300-clean.sh
> +++ b/t/t7300-clean.sh
> @@ -332,6 +332,13 @@ test_expect_success 'git clean -d -X' '
>
> '
>
> +test_expect_failure 'git clean -d -X' '
> + mkdir -p a/b &&
> + touch a/b/c.o &&
> + git clean -d -X &&
> + ! test -d a
Thanks for the test case. BTW, you might consider changing this last
line to "! test -f a/b/c.o". Reasoning: it is clear from the docs
that c.o should be removed by "git clean -X" (since c.o is an ignored
file). It is less clear whether the directories a and a/b should be
removed by "git clean -X", since those directories are not in
themselves ignored, only their contents.
> +'
> +
> test_expect_success 'clean.requireForce defaults to true' '
>
> git config --unset clean.requireForce &&
> --
> 1.7.9.174.g356eff6
^ permalink raw reply
* Re: [BUG] git clean -X skips a directory containing only ignored files
From: Andrew Wong @ 2012-01-31 16:20 UTC (permalink / raw)
To: Paul Berry; +Cc: git
In-Reply-To: <CA+yLL67J-7U9z7HVvq5wTc1g4_UCtqYfEyqdt7XR5zDqvQN5NA@mail.gmail.com>
I think there were a bit of discussions on this issues just while ago too:
http://thread.gmane.org/gmane.comp.version-control.git/188605
On 01/30/2012 06:36 PM, Paul Berry wrote:
> I am trying to use "git clean -X" to remove object files (which
> are gitignored) from my source tree, but it appears to miss
> object files that are in a subdirectory without any git-tracked
> files:
>
> $ git init test
> Initialized empty Git repository in /home/pberry/tmp/test/.git/
> $ cd test
> $ mkdir foo
> $ touch foo/bar.o
> $ echo '*.o' > .gitignore
> $ git add .gitignore
> $ git commit -mgitignore
> [master (root-commit) 6b5ffcb] gitignore
> 1 files changed, 1 insertions(+), 0 deletions(-)
> create mode 100644 .gitignore
> $ git status
> # On branch master
> nothing to commit (working directory clean)
> $ git clean -d -X -f
> $ ls foo
> bar.o
>
> It seems to me that bar.o should have been removed, because
> according to the git-clean docs, -X means "Remove only files
> ignored by git", and bar.o is definitely being ignored by git.
>
>
> It looks like a very similar bug was reported back in 2010, but
> not fixed:
> http://git.661346.n2.nabble.com/BUG-git-clean-X-behaviour-when-gitignore-has-sub-directory-entries-td5575307.html.
> I've confirmed that the workaround mentioned by Jonathan Nieder
> in that thread fixes my problem too (removing "dir.flags |=
> DIR_SHOW_OTHER_DIRECTORIES;" from builtin/clean.c). However I'm
> guessing from Jonathan's comments that it would be better to fix
> this bug elsewhere (somewhere in dir.c perhaps).
>
> Is anyone interested in following up on this old bug?
> Alternatively, if someone could give me some guidance as to the
> best way to go about fixing this bug, I would be glad to submit a
> patch.
>
> Thanks,
>
> Paul
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: Bug: "git checkout -b" should be allowed in empty repo
From: Michael Haggerty @ 2012-01-31 16:32 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Junio C Hamano, git, demerphq
In-Reply-To: <m3k448che9.fsf@localhost.localdomain>
On 01/31/2012 11:09 AM, Jakub Narebski wrote:
> I don't see how this can be done in backward-compatibile way.
Yes, backwards compatibility would probably prevent the NULL commit idea
from ever being implemented in a literal way.
But it is conceivable that it could be faked with some strategic
if sha1 == '0'*40:
treat_as_special_null_commit
elif len(parents) == 0:
parents = ['0'*40]
In other words, include a little special case fakery in the data
structures near root commits (an O(1) amount of work) to avoid special
cases in all commands that can touch root commits (an O(number of
commands) amount of work).
Alternatively, the NULL commit could be a UI construct that has no
manifestation in the object model. This would not save implementation
work, but would perhaps give a more consistent way to deal with root
commits in the UI than the current array of --orphan etc. options.
> Please note that in Git it is quite natural to have more than one root
> (parentless) commit, even without presence of disconnected / orphan
> branches. They are result of joining originally separate projects.
> git.git has quite a few of them (more than 6, IIRC).
I don't see the problem, unless you mean that it would be difficult to
merge repositories that don't link back to a NULL commit with
hypothetical future repositories that do include a NULL commit. But a
world in which two kinds of repositories have to be supported is
pointless anyway, because then the git code would have to include *both*
kinds of special cases and nothing would be gained.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: [PATCH] Correct singular form in diff summary line for human interaction
From: Jonathan Nieder @ 2012-01-31 15:20 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy
Cc: git, Ævar Arnfjörð Bjarmason, Frederik Schwarzer,
Junio C Hamano
In-Reply-To: <1328019840-6168-1-git-send-email-pclouds@gmail.com>
Hi,
Nguyễn Thái Ngọc Duy wrote:
> Convenient function interactive_use() is added for this purpose. The
> command thinks it's in human hands when:
I admit I really dislike this, especially:
> - GIT_SCRIPTING environment variable is not set
If my GUI app was parsing diffstats to convert them into a visual
representation, as a novice it may not be obvious to me where to find
the menu entry file to set this envvar in.
But maybe I'm not the right person to ask, since I'd be okay with
removing the "s"es (with an appropriate incubation time to discover
whether we are introducing a regression) unconditionally.
If there is an environment variable to say "I don't want to see
variations on strings intended for humans", can it be spelled as
LC_ALL=C?
Just my two cents,
Jonathan
^ permalink raw reply
* Re: [BUG] git clean -X skips a directory containing only ignored files
From: Michael Schubert @ 2012-01-31 14:47 UTC (permalink / raw)
To: Paul Berry; +Cc: git
In-Reply-To: <CA+yLL67J-7U9z7HVvq5wTc1g4_UCtqYfEyqdt7XR5zDqvQN5NA@mail.gmail.com>
On 01/31/2012 12:36 AM, Paul Berry wrote:
> I am trying to use "git clean -X" to remove object files (which
> are gitignored) from my source tree, but it appears to miss
> object files that are in a subdirectory without any git-tracked
> files:
>
> $ git init test
> Initialized empty Git repository in /home/pberry/tmp/test/.git/
> $ cd test
> $ mkdir foo
> $ touch foo/bar.o
> $ echo '*.o' > .gitignore
> $ git add .gitignore
> $ git commit -mgitignore
> [master (root-commit) 6b5ffcb] gitignore
> 1 files changed, 1 insertions(+), 0 deletions(-)
> create mode 100644 .gitignore
> $ git status
> # On branch master
> nothing to commit (working directory clean)
> $ git clean -d -X -f
> $ ls foo
> bar.o
>
> It seems to me that bar.o should have been removed, because
> according to the git-clean docs, -X means "Remove only files
> ignored by git", and bar.o is definitely being ignored by git.
>
>
> It looks like a very similar bug was reported back in 2010, but
> not fixed:
> http://git.661346.n2.nabble.com/BUG-git-clean-X-behaviour-when-gitignore-has-sub-directory-entries-td5575307.html.
> I've confirmed that the workaround mentioned by Jonathan Nieder
> in that thread fixes my problem too (removing "dir.flags |=
> DIR_SHOW_OTHER_DIRECTORIES;" from builtin/clean.c). However I'm
> guessing from Jonathan's comments that it would be better to fix
> this bug elsewhere (somewhere in dir.c perhaps).
Removing DIR_SHOW_OTHER_DIRECTORIES just happens to not trigger
this particular "bug" but breaks pretty much everything else.
As a workaround, you could explicitly add the directory to your
gitignore file.
Here's a test:
-- >8 --
Subject: [PATCH] t7300-clean: show known breakage with "git clean -d -X"
"git clean -d -X" fails for directories containing only untracked files.
Example:
$ ls -R .
.:
foo
./foo:
bar.o
$ cat .gitignore
*.o
$ git clean -d -X -f
$ ! test -d foo || echo fail
Reported-by: Paul Berry <stereotype441@gmail.com>
Signed-off-by: Michael Schubert <mschub@elegosoft.com>
---
t/t7300-clean.sh | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
index 800b536..0b6d545 100755
--- a/t/t7300-clean.sh
+++ b/t/t7300-clean.sh
@@ -332,6 +332,13 @@ test_expect_success 'git clean -d -X' '
'
+test_expect_failure 'git clean -d -X' '
+ mkdir -p a/b &&
+ touch a/b/c.o &&
+ git clean -d -X &&
+ ! test -d a
+'
+
test_expect_success 'clean.requireForce defaults to true' '
git config --unset clean.requireForce &&
--
1.7.9.174.g356eff6
^ permalink raw reply related
* [PATCH/RFC v3] grep: Add the option '--exclude'
From: Albert Yale @ 2012-01-31 14:45 UTC (permalink / raw)
To: git; +Cc: gitster, Albert Yale
[PATCH/RFC v3] grep: Add the option '--exclude'
Signed-off-by: Albert Yale <surfingalbert@gmail.com>
---
Here's my latest progress towards a cleaner implementation.
Albert Yale
Documentation/git-grep.txt | 7 +++
builtin/grep.c | 41 +++++++++++++--
cache.h | 3 +
dir.c | 126 +++++++++++++++++++++++++++++++++++++------
dir.h | 3 +-
read-cache.c | 2 +-
6 files changed, 158 insertions(+), 24 deletions(-)
diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index 6a8b1e3..67a83aa 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -22,6 +22,7 @@ SYNOPSIS
[--color[=<when>] | --no-color]
[-A <post-context>] [-B <pre-context>] [-C <context>]
[-f <file>] [-e] <pattern>
+ [-x <pattern>|--exclude <pattern>]
[--and|--or|--not|(|)|-e <pattern>...]
[ [--exclude-standard] [--cached | --no-index | --untracked] | <tree>...]
[--] [<pathspec>...]
@@ -124,6 +125,12 @@ OPTIONS
Use fixed strings for patterns (don't interpret pattern
as a regex).
+-x <pattern>::
+--exclude <pattern>::
+ In addition to those found in .gitignore (per directory) and
+ $GIT_DIR/info/exclude, also consider these patterns to be in the
+ set of the ignore rules in effect.
+
-n::
--line-number::
Prefix the line number to matching lines.
diff --git a/builtin/grep.c b/builtin/grep.c
index 9ce064a..e9e1ac1 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -528,7 +528,8 @@ static int grep_cache(struct grep_opt *opt, const struct pathspec *pathspec, int
struct cache_entry *ce = active_cache[nr];
if (!S_ISREG(ce->ce_mode))
continue;
- if (!match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, NULL))
+ if (!match_pathspec_depth(pathspec, ce->name, ce_namelen(ce),
+ 0, NULL, 1))
continue;
/*
* If CE_VALID is on, we assume worktree file and its cache entry
@@ -566,6 +567,11 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
while (tree_entry(tree, &entry)) {
int te_len = tree_entry_len(&entry);
+ if (match_pathspec_depth(pathspec->exclude,
+ entry.path, strlen(entry.path),
+ 0, NULL, 0))
+ continue;
+
if (match != all_entries_interesting) {
match = tree_entry_interesting(&entry, base, tn_len, pathspec);
if (match == all_entries_not_interesting)
@@ -606,8 +612,17 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec,
struct object *obj, const char *name)
{
- if (obj->type == OBJ_BLOB)
+ if (obj->type == OBJ_BLOB) {
+ const char *name_without_sha1 = strchr(name, ':') + 1;
+
+ if (match_pathspec_depth(pathspec->exclude,
+ name_without_sha1,
+ strlen(name_without_sha1),
+ 0, NULL, 0))
+ return 0;
+
return grep_sha1(opt, obj->sha1, name, 0);
+ }
if (obj->type == OBJ_COMMIT || obj->type == OBJ_TREE) {
struct tree_desc tree;
void *data;
@@ -671,7 +686,7 @@ static int grep_directory(struct grep_opt *opt, const struct pathspec *pathspec,
for (i = 0; i < dir.nr; i++) {
const char *name = dir.entries[i]->name;
int namelen = strlen(name);
- if (!match_pathspec_depth(pathspec, name, namelen, 0, NULL))
+ if (!match_pathspec_depth(pathspec, name, namelen, 0, NULL, 1))
continue;
hit |= grep_file(opt, dir.entries[i]->name);
if (hit && opt->status_only)
@@ -764,6 +779,14 @@ static int pattern_callback(const struct option *opt, const char *arg,
return 0;
}
+static int exclude_cb(const struct option *opt, const char *arg,
+ int unset)
+{
+ struct string_list *exclude_list = opt->value;
+ string_list_append(exclude_list, arg);
+ return 0;
+}
+
static int help_callback(const struct option *opt, const char *arg, int unset)
{
return -1;
@@ -792,7 +815,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
pattern_type_pcre,
};
int pattern_type = pattern_type_unspecified;
-
+ struct string_list exclude_list = STRING_LIST_INIT_NODUP;
struct option options[] = {
OPT_BOOLEAN(0, "cached", &cached,
"search in index instead of in the work tree"),
@@ -872,6 +895,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
"read patterns from file", file_callback),
{ OPTION_CALLBACK, 'e', NULL, &opt, "pattern",
"match <pattern>", PARSE_OPT_NONEG, pattern_callback },
+ { OPTION_CALLBACK, 'x', "exclude", &exclude_list, "pattern",
+ "add <pattern> to ignore rules", PARSE_OPT_NONEG, exclude_cb },
{ OPTION_CALLBACK, 0, "and", &opt, NULL,
"combine patterns specified with -e",
PARSE_OPT_NOARG | PARSE_OPT_NONEG, and_callback },
@@ -1053,6 +1078,12 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
pathspec.max_depth = opt.max_depth;
pathspec.recursive = 1;
+ if (exclude_list.nr) {
+ add_pathspec_item_from_string_list(pathspec.exclude, &exclude_list);
+ pathspec.exclude->max_depth = opt.max_depth;
+ pathspec.exclude->recursive = 1;
+ }
+
if (show_in_pager && (cached || list.nr))
die(_("--open-files-in-pager only works on the worktree"));
@@ -1102,5 +1133,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
if (hit && show_in_pager)
run_pager(&opt, prefix);
free_grep_patterns(&opt);
+ free_pathspec(&pathspec);
+ string_list_clear(&exclude_list, 0);
return !hit;
}
diff --git a/cache.h b/cache.h
index 9bd8c2d..683458a 100644
--- a/cache.h
+++ b/cache.h
@@ -533,9 +533,12 @@ struct pathspec {
int len;
unsigned int use_wildcard:1;
} *items;
+
+ struct pathspec *exclude; /* This is never NULL. */
};
extern int init_pathspec(struct pathspec *, const char **);
+extern int add_pathspec_item_from_string_list(struct pathspec *, const struct string_list *);
extern void free_pathspec(struct pathspec *);
extern int ce_path_match(const struct cache_entry *ce, const struct pathspec *pathspec);
diff --git a/dir.c b/dir.c
index 0a78d00..9e4312c 100644
--- a/dir.c
+++ b/dir.c
@@ -9,6 +9,8 @@
#include "dir.h"
#include "refs.h"
+#include "string-list.h"
+
struct path_simplify {
int len;
const char *path;
@@ -253,34 +255,47 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix,
*/
int match_pathspec_depth(const struct pathspec *ps,
const char *name, int namelen,
- int prefix, char *seen)
+ int prefix, char *seen,
+ int empty_pathspec_can_match)
{
int i, retval = 0;
+ const char *name_without_prefix = name + prefix;
+ int namelen_without_prefix = namelen - prefix;
if (!ps->nr) {
- if (!ps->recursive || ps->max_depth == -1)
- return MATCHED_RECURSIVELY;
+ if (empty_pathspec_can_match) {
+ if (match_pathspec_depth(ps->exclude,
+ name, namelen,
+ prefix, seen, 0))
+ return 0;
- if (within_depth(name, namelen, 0, ps->max_depth))
- return MATCHED_EXACTLY;
+ if (!ps->recursive || ps->max_depth == -1)
+ return MATCHED_RECURSIVELY;
+
+ if (within_depth(name, namelen, 0, ps->max_depth))
+ return MATCHED_EXACTLY;
+ else
+ return 0;
+ }
else
return 0;
}
- name += prefix;
- namelen -= prefix;
-
for (i = ps->nr - 1; i >= 0; i--) {
int how;
if (seen && seen[i] == MATCHED_EXACTLY)
continue;
- how = match_pathspec_item(ps->items+i, prefix, name, namelen);
+ how = match_pathspec_item(ps->items+i, prefix,
+ name_without_prefix,
+ namelen_without_prefix);
if (ps->recursive && ps->max_depth != -1 &&
how && how != MATCHED_FNMATCH) {
int len = ps->items[i].len;
- if (name[len] == '/')
+ if (name_without_prefix[len] == '/')
len++;
- if (within_depth(name+len, namelen-len, 0, ps->max_depth))
+ if (within_depth(name_without_prefix+len,
+ namelen_without_prefix-len,
+ 0, ps->max_depth))
how = MATCHED_EXACTLY;
else
how = 0;
@@ -292,6 +307,12 @@ int match_pathspec_depth(const struct pathspec *ps,
seen[i] = how;
}
}
+
+ if (retval && match_pathspec_depth(ps->exclude,
+ name, namelen,
+ prefix, seen, 0))
+ return 0;
+
return retval;
}
@@ -1259,12 +1280,79 @@ static int pathspec_item_cmp(const void *a_, const void *b_)
return strcmp(a->match, b->match);
}
+static int init_pathspec_item(struct pathspec_item *item, const char *path)
+{
+ item->match = path;
+ item->len = strlen(path);
+ item->use_wildcard = !no_wildcard(path);
+
+ return item->use_wildcard;
+}
+
+int add_pathspec_item_from_string_list(struct pathspec *pathspec,
+ const struct string_list *path_list)
+{
+ int i;
+ struct pathspec_item *new_items;
+ int previous_items_count = pathspec->nr;
+
+ if (!path_list->nr)
+ return 0;
+
+ pathspec->nr += path_list->nr;
+
+ if (previous_items_count)
+ pathspec->items = xrealloc(pathspec->items,
+ sizeof(struct pathspec_item)*pathspec->nr);
+ else
+ pathspec->items = xcalloc(pathspec->nr, sizeof(struct pathspec_item));
+
+ new_items = pathspec->items + previous_items_count;
+
+ for (i = 0; i < path_list->nr; i++) {
+ struct pathspec_item *item = new_items+i;
+ const char *path = path_list->items[i].string;
+
+ pathspec->has_wildcard |= init_pathspec_item(item, path);
+ }
+
+ /*
+ * Give the pathspec a pathspec->exclude so we can test
+ * pathspec->exclude->nr without testing the validity of
+ * pathspec->exclude first.
+ */
+ if (!pathspec->exclude)
+ pathspec->exclude = xcalloc(1, sizeof(*pathspec));
+
+ qsort(pathspec->items, pathspec->nr,
+ sizeof(struct pathspec_item), pathspec_item_cmp);
+
+ /*
+ * Please comment/review:
+ *
+ * Here, pathspec->raw is NULL despite pathspec->nr being non-zero.
+ *
+ * Usually, a NULL pathspec->raw implies that pathspec->nr is zero.
+ *
+ * A problem might arrise if other areas of the code assume that a
+ * NULL pathspec->raw means that pathspec->nr is zero, or vice-versa.
+ */
+
+ return 0;
+}
+
int init_pathspec(struct pathspec *pathspec, const char **paths)
{
const char **p = paths;
int i;
memset(pathspec, 0, sizeof(*pathspec));
+ /*
+ * We create pathspec->exclude unconditionally so that we don't have
+ * to test the validity of pathspec->exclude before using the value
+ * of pathspec->exclude->nr.
+ */
+ pathspec->exclude = xcalloc(1, sizeof(*pathspec));
if (!p)
return 0;
while (*p)
@@ -1279,11 +1367,7 @@ int init_pathspec(struct pathspec *pathspec, const char **paths)
struct pathspec_item *item = pathspec->items+i;
const char *path = paths[i];
- item->match = path;
- item->len = strlen(path);
- item->use_wildcard = !no_wildcard(path);
- if (item->use_wildcard)
- pathspec->has_wildcard = 1;
+ pathspec->has_wildcard |= init_pathspec_item(item, path);
}
qsort(pathspec->items, pathspec->nr,
@@ -1294,6 +1378,12 @@ int init_pathspec(struct pathspec *pathspec, const char **paths)
void free_pathspec(struct pathspec *pathspec)
{
- free(pathspec->items);
- pathspec->items = NULL;
+ if (pathspec->items) {
+ free(pathspec->items);
+ pathspec->items = NULL;
+ }
+ if (pathspec->exclude) {
+ free_pathspec(pathspec->exclude);
+ pathspec->exclude = NULL;
+ }
}
diff --git a/dir.h b/dir.h
index dd6947e..2b9870d 100644
--- a/dir.h
+++ b/dir.h
@@ -68,7 +68,8 @@ extern char *common_prefix(const char **pathspec);
extern int match_pathspec(const char **pathspec, const char *name, int namelen, int prefix, char *seen);
extern int match_pathspec_depth(const struct pathspec *pathspec,
const char *name, int namelen,
- int prefix, char *seen);
+ int prefix, char *seen,
+ int empty_pathspec_can_match);
extern int within_depth(const char *name, int namelen, int depth, int max_depth);
extern int fill_directory(struct dir_struct *dir, const char **pathspec);
diff --git a/read-cache.c b/read-cache.c
index a51bba1..9783c4f 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -708,7 +708,7 @@ int ce_same_name(struct cache_entry *a, struct cache_entry *b)
int ce_path_match(const struct cache_entry *ce, const struct pathspec *pathspec)
{
- return match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, NULL);
+ return match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, NULL, 1);
}
/*
--
1.7.8.3
^ permalink raw reply related
* Alternates corruption issue
From: Richard Purdie @ 2012-01-31 14:05 UTC (permalink / raw)
To: GIT Mailing-list; +Cc: Hart, Darren, Ashfield, Bruce
I have a problem with git clone commands using alternates failing by
mixing up different repositories. I have a situation where I could end
up with both:
/srv/mirrors/repo
/srv/mirrors/repo.git
as bare clones.
I then try cloning "repo" with alternates with the command:
$ git clone -s -n /srv/mirrors/repo /tmp/foo
Cloning into /tmp/foo...
done.
$ cat /tmp/foo/.git/objects/info/alternates
/srv/mirrors/repo.git/objects
Note how I'm now referencing repo.git, not repo. This doesn't work as
expected giving some very bizarre results when actually using the
repository.
I appreciate this is a rather bizarre corner case but its one that is
breaking the build system I work with. Ideally people would use a
consistent URL for the same repository but we have an example where they
haven't and this really shouldn't break like this.
Looking at the code, the cause seems to be
clone.c:get_repo_path():
static char *suffix[] = { "/.git", ".git", "" };
since its looking in order for:
repo/.git (fails)
repo.git (suceeds, incorrect)
repo (never looked at)
I'm not sure what would break if that order were to change, swapping the
last two options.
I can "force" the issue by running:
git clone -s -n /srv/mirrors/repo/ /tmp/foo
but this results in the slightly odd looking:
$ cat /tmp/foo/.git/objects/info/alternates
/srv/mirrors/repo//objects
which does at least work.
Any idea if its possible to fix the root cause of this problem?
Cheers,
Richard
^ permalink raw reply
* [PATCH] Correct singular form in diff summary line for human interaction
From: Nguyễn Thái Ngọc Duy @ 2012-01-31 14:24 UTC (permalink / raw)
To: git
Cc: Ævar Arnfjörð Bjarmason, Frederik Schwarzer,
Junio C Hamano, Nguyễn Thái Ngọc Duy
"git diff --stat" and "git apply --stat" now learn to print the line
"%d files changed, %d insertions(+), %d deletions(-)" in singular form
whenever applicable.
This change uncondtionally would upset scripts because scripts are not
nice. They simply hate changes. They can be very hostile when that
happens. So only adjust the line for human consumption.
Convenient function interactive_use() is added for this purpose. The
command thinks it's in human hands when:
- GIT_SCRIPTING environment variable is not set
- pager is on or
- both stdout and stderr point to tty device
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
The third attempt in the last couple months to remove three "s" in
one line. I did not upset the test suite with this and hopefully
won't upset any scripts around. Good start?
builtin/apply.c | 3 ++-
cache.h | 1 +
diff.c | 33 ++++++++++++++++++++++++++++-----
diff.h | 3 +++
| 7 +++++++
5 files changed, 41 insertions(+), 6 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index c24dc54..389898f 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -14,6 +14,7 @@
#include "builtin.h"
#include "string-list.h"
#include "dir.h"
+#include "diff.h"
#include "parse-options.h"
/*
@@ -3241,7 +3242,7 @@ static void stat_patch_list(struct patch *patch)
show_stats(patch);
}
- printf(" %d files changed, %d insertions(+), %d deletions(-)\n", files, adds, dels);
+ print_stat_summary(stdout, files, adds, dels);
}
static void numstat_patch_list(struct patch *patch)
diff --git a/cache.h b/cache.h
index 10afd71..7e0bb2b 100644
--- a/cache.h
+++ b/cache.h
@@ -1175,6 +1175,7 @@ extern void setup_pager(void);
extern const char *pager_program;
extern int pager_in_use(void);
extern int pager_use_color;
+extern int interactive_use(void);
extern const char *editor_program;
extern const char *askpass_program;
diff --git a/diff.c b/diff.c
index 7e15426..2d63e9c 100644
--- a/diff.c
+++ b/diff.c
@@ -1322,6 +1322,32 @@ static void fill_print_name(struct diffstat_file *file)
file->print_name = pname;
}
+int print_stat_summary(FILE *fp, int files, int insertions, int deletions)
+{
+ struct strbuf sb = STRBUF_INIT;
+ int ret;
+
+ if (!interactive_use())
+ return fprintf(fp, " %d files changed, %d insertions(+), %d deletions(-)\n",
+ files, insertions, deletions);
+
+ strbuf_addf(&sb,
+ ngettext(" %d file changed,", " %d files changed,",
+ files),
+ files);
+ strbuf_addf(&sb,
+ ngettext(" %d insertion(+),", " %d insertions(+),",
+ insertions),
+ insertions);
+ strbuf_addf(&sb,
+ ngettext(" %d deletion(-)\n", " %d deletions(-)\n",
+ deletions),
+ deletions);
+ ret = fputs(sb.buf, fp);
+ strbuf_release(&sb);
+ return ret;
+}
+
static void show_stats(struct diffstat_t *data, struct diff_options *options)
{
int i, len, add, del, adds = 0, dels = 0;
@@ -1475,9 +1501,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
extra_shown = 1;
}
fprintf(options->file, "%s", line_prefix);
- fprintf(options->file,
- " %d files changed, %d insertions(+), %d deletions(-)\n",
- total_files, adds, dels);
+ print_stat_summary(options->file, total_files, adds, dels);
}
static void show_shortstats(struct diffstat_t *data, struct diff_options *options)
@@ -1507,8 +1531,7 @@ static void show_shortstats(struct diffstat_t *data, struct diff_options *option
options->output_prefix_data);
fprintf(options->file, "%s", msg->buf);
}
- fprintf(options->file, " %d files changed, %d insertions(+), %d deletions(-)\n",
- total_files, adds, dels);
+ print_stat_summary(options->file, total_files, adds, dels);
}
static void show_numstat(struct diffstat_t *data, struct diff_options *options)
diff --git a/diff.h b/diff.h
index ae71f4c..7af5f1e 100644
--- a/diff.h
+++ b/diff.h
@@ -324,4 +324,7 @@ extern struct userdiff_driver *get_textconv(struct diff_filespec *one);
extern int parse_rename_score(const char **cp_p);
+extern int print_stat_summary(FILE *fp, int files,
+ int insertions, int deletions);
+
#endif /* DIFF_H */
--git a/pager.c b/pager.c
index 975955b..9a3d3d8 100644
--- a/pager.c
+++ b/pager.c
@@ -110,3 +110,10 @@ int pager_in_use(void)
env = getenv("GIT_PAGER_IN_USE");
return env ? git_config_bool("GIT_PAGER_IN_USE", env) : 0;
}
+
+int interactive_use(void)
+{
+ const char *env;
+ env = getenv("GIT_SCRIPTING");
+ return !env && (pager_in_use() || (isatty(1) && isatty(2)));
+}
--
1.7.8.36.g69ee2
^ permalink raw reply related
* [PATCH] pack-objects: convert to use parse_options()
From: Nguyễn Thái Ngọc Duy @ 2012-01-31 13:48 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/pack-objects.c | 317 +++++++++++++++++++++---------------------------
1 files changed, 141 insertions(+), 176 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 0f2e7b8..59fdb3f 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -18,16 +18,11 @@
#include "refs.h"
#include "thread-utils.h"
-static const char pack_usage[] =
- "git pack-objects [ -q | --progress | --all-progress ]\n"
- " [--all-progress-implied]\n"
- " [--max-pack-size=<n>] [--local] [--incremental]\n"
- " [--window=<n>] [--window-memory=<n>] [--depth=<n>]\n"
- " [--no-reuse-delta] [--no-reuse-object] [--delta-base-offset]\n"
- " [--threads=<n>] [--non-empty] [--revs [--unpacked | --all]]\n"
- " [--reflog] [--stdout | base-name] [--include-tag]\n"
- " [--keep-unreachable | --unpack-unreachable]\n"
- " [< ref-list | < object-list]";
+static const char *pack_usage[] = {
+ "git pack-objects --stdout [options...] [< ref-list | < object-list]",
+ "git pack-objects [options...] base-name [< ref-list | < object-list]",
+ NULL
+};
struct object_entry {
struct pack_idx_entry idx;
@@ -2305,183 +2300,140 @@ static void get_object_list(int ac, const char **av)
loosen_unused_packed_objects(&revs);
}
+static int option_parse_index_version(const struct option *opt,
+ const char *arg, int unset)
+{
+ char *c;
+ const char *val = arg;
+ pack_idx_opts.version = strtoul(val, &c, 10);
+ if (pack_idx_opts.version > 2)
+ die("unsupported index version %s", val);
+ if (*c == ',' && c[1])
+ pack_idx_opts.off32_limit = strtoul(c+1, &c, 0);
+ if (*c || pack_idx_opts.off32_limit & 0x80000000)
+ die("bad index version %s", val);
+ return 0;
+}
+
+static int option_parse_ulong(const struct option *opt,
+ const char *arg, int unset)
+{
+ if (unset)
+ die("option %s does not accept negative form",
+ opt->long_name);
+
+ if (!git_parse_ulong(arg, opt->value))
+ die("unable to parse value '%s' for option %s",
+ arg, opt->long_name);
+ return 0;
+}
+
+#define OPT_ULONG(s, l, v, h) \
+ { OPTION_CALLBACK, (s), (l), (v), "n", (h), \
+ PARSE_OPT_NONEG, option_parse_ulong }
+
int cmd_pack_objects(int argc, const char **argv, const char *prefix)
{
int use_internal_rev_list = 0;
int thin = 0;
int all_progress_implied = 0;
- uint32_t i;
- const char **rp_av;
- int rp_ac_alloc = 64;
- int rp_ac;
+ const char *rp_av[6];
+ int rp_ac = 0;
+ int rev_list_unpacked = 0, rev_list_all = 0, rev_list_reflog = 0;
+ struct option pack_objects_options[] = {
+ OPT_SET_INT('q', "quiet", &progress,
+ "do not show progress meter", 0),
+ OPT_SET_INT(0, "progress", &progress,
+ "show progress meter", 1),
+ OPT_SET_INT(0, "all-progress", &progress,
+ "show progress meter during object writing phase", 2),
+ OPT_BOOL(0, "all-progress-implied",
+ &all_progress_implied,
+ "similar to --all-progress when progress meter is shown"),
+ { OPTION_CALLBACK, 0, "index-version", NULL, "version",
+ "force generating pack index at a particular version",
+ 0, option_parse_index_version },
+ OPT_ULONG(0, "max-pack-size", &pack_size_limit,
+ "maximum size of each output pack file"),
+ OPT_BOOL(0, "local", &local,
+ "ignore borrowed objects from alternate object store"),
+ OPT_BOOL(0, "incremental", &incremental,
+ "ignore packed objects"),
+ OPT_INTEGER(0, "window", &window,
+ "limit pack window by objects"),
+ OPT_ULONG(0, "window-memory", &window_memory_limit,
+ "limit pack window by memory"),
+ OPT_INTEGER(0, "depth", &depth,
+ "limit pack window by maximum delta depth"),
+ OPT_BOOL(0, "reuse-delta", &reuse_delta,
+ "reusing existing deltas"),
+ OPT_BOOL(0, "reuse-object", &reuse_object,
+ "reusing existing objects"),
+ OPT_BOOL(0, "delta-base-offset", &allow_ofs_delta,
+ "use OFS_DELTA objects"),
+ OPT_INTEGER(0, "threads", &delta_search_threads,
+ "use threads when searching for best delta matches"),
+ OPT_BOOL(0, "non-empty", &non_empty,
+ "only create if it would contain at least one object"),
+ OPT_BOOL(0, "revs", &use_internal_rev_list,
+ "read revision arguments from standard output"),
+ { OPTION_SET_INT, 0, "unpacked", &rev_list_unpacked, NULL,
+ "limit the objects to those that are not already packed",
+ PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },
+ { OPTION_SET_INT, 0, "all", &rev_list_all, NULL,
+ "include all refs under $GIT_DIR/refs directory",
+ PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },
+ { OPTION_SET_INT, 0, "reflog", &rev_list_reflog, NULL,
+ "include objects referred by reflog entries",
+ PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },
+ OPT_BOOL(0, "stdout", &pack_to_stdout,
+ "output pack to stdout"),
+ OPT_BOOL(0, "include-tag", &include_tag,
+ "include unasked-for annotated tags"),
+ OPT_BOOL(0, "keep-unreachable", &keep_unreachable,
+ "keep unreachable objects"),
+ OPT_BOOL(0, "unpack-unreachable", &unpack_unreachable,
+ "unpack unreachable objects"),
+ OPT_BOOL(0, "thin", &thin,
+ "create thin packs"),
+ OPT_BOOL(0, "honor-pack-keep", &ignore_packed_keep,
+ "ignore packs that have companion .keep file"),
+ OPT_INTEGER(0, "compression", &pack_compression_level,
+ "pack compression level"),
+ OPT_SET_INT(0, "keep-true-parents", &grafts_replace_parents,
+ "do not hide commits by grafts", 0),
+ OPT_END(),
+ };
read_replace_refs = 0;
- rp_av = xcalloc(rp_ac_alloc, sizeof(*rp_av));
-
- rp_av[0] = "pack-objects";
- rp_av[1] = "--objects"; /* --thin will make it --objects-edge */
- rp_ac = 2;
-
reset_pack_idx_option(&pack_idx_opts);
git_config(git_pack_config, NULL);
if (!pack_compression_seen && core_compression_seen)
pack_compression_level = core_compression_level;
progress = isatty(2);
- for (i = 1; i < argc; i++) {
- const char *arg = argv[i];
+ argc = parse_options(argc, argv, prefix, pack_objects_options,
+ pack_usage, 0);
- if (*arg != '-')
- break;
+ rp_av[rp_ac++] = "pack-objects";
+ if (thin) {
+ use_internal_rev_list = 1;
+ rp_av[rp_ac++] = "--objects-edge";
+ } else
+ rp_av[rp_ac++] = "--objects";
- if (!strcmp("--non-empty", arg)) {
- non_empty = 1;
- continue;
- }
- if (!strcmp("--local", arg)) {
- local = 1;
- continue;
- }
- if (!strcmp("--incremental", arg)) {
- incremental = 1;
- continue;
- }
- if (!strcmp("--honor-pack-keep", arg)) {
- ignore_packed_keep = 1;
- continue;
- }
- if (!prefixcmp(arg, "--compression=")) {
- char *end;
- int level = strtoul(arg+14, &end, 0);
- if (!arg[14] || *end)
- usage(pack_usage);
- if (level == -1)
- level = Z_DEFAULT_COMPRESSION;
- else if (level < 0 || level > Z_BEST_COMPRESSION)
- die("bad pack compression level %d", level);
- pack_compression_level = level;
- continue;
- }
- if (!prefixcmp(arg, "--max-pack-size=")) {
- pack_size_limit_cfg = 0;
- if (!git_parse_ulong(arg+16, &pack_size_limit))
- usage(pack_usage);
- continue;
- }
- if (!prefixcmp(arg, "--window=")) {
- char *end;
- window = strtoul(arg+9, &end, 0);
- if (!arg[9] || *end)
- usage(pack_usage);
- continue;
- }
- if (!prefixcmp(arg, "--window-memory=")) {
- if (!git_parse_ulong(arg+16, &window_memory_limit))
- usage(pack_usage);
- continue;
- }
- if (!prefixcmp(arg, "--threads=")) {
- char *end;
- delta_search_threads = strtoul(arg+10, &end, 0);
- if (!arg[10] || *end || delta_search_threads < 0)
- usage(pack_usage);
-#ifdef NO_PTHREADS
- if (delta_search_threads != 1)
- warning("no threads support, "
- "ignoring %s", arg);
-#endif
- continue;
- }
- if (!prefixcmp(arg, "--depth=")) {
- char *end;
- depth = strtoul(arg+8, &end, 0);
- if (!arg[8] || *end)
- usage(pack_usage);
- continue;
- }
- if (!strcmp("--progress", arg)) {
- progress = 1;
- continue;
- }
- if (!strcmp("--all-progress", arg)) {
- progress = 2;
- continue;
- }
- if (!strcmp("--all-progress-implied", arg)) {
- all_progress_implied = 1;
- continue;
- }
- if (!strcmp("-q", arg)) {
- progress = 0;
- continue;
- }
- if (!strcmp("--no-reuse-delta", arg)) {
- reuse_delta = 0;
- continue;
- }
- if (!strcmp("--no-reuse-object", arg)) {
- reuse_object = reuse_delta = 0;
- continue;
- }
- if (!strcmp("--delta-base-offset", arg)) {
- allow_ofs_delta = 1;
- continue;
- }
- if (!strcmp("--stdout", arg)) {
- pack_to_stdout = 1;
- continue;
- }
- if (!strcmp("--revs", arg)) {
- use_internal_rev_list = 1;
- continue;
- }
- if (!strcmp("--keep-unreachable", arg)) {
- keep_unreachable = 1;
- continue;
- }
- if (!strcmp("--unpack-unreachable", arg)) {
- unpack_unreachable = 1;
- continue;
- }
- if (!strcmp("--include-tag", arg)) {
- include_tag = 1;
- continue;
- }
- if (!strcmp("--unpacked", arg) ||
- !strcmp("--reflog", arg) ||
- !strcmp("--all", arg)) {
- use_internal_rev_list = 1;
- if (rp_ac >= rp_ac_alloc - 1) {
- rp_ac_alloc = alloc_nr(rp_ac_alloc);
- rp_av = xrealloc(rp_av,
- rp_ac_alloc * sizeof(*rp_av));
- }
- rp_av[rp_ac++] = arg;
- continue;
- }
- if (!strcmp("--thin", arg)) {
- use_internal_rev_list = 1;
- thin = 1;
- rp_av[1] = "--objects-edge";
- continue;
- }
- if (!prefixcmp(arg, "--index-version=")) {
- char *c;
- pack_idx_opts.version = strtoul(arg + 16, &c, 10);
- if (pack_idx_opts.version > 2)
- die("bad %s", arg);
- if (*c == ',')
- pack_idx_opts.off32_limit = strtoul(c+1, &c, 0);
- if (*c || pack_idx_opts.off32_limit & 0x80000000)
- die("bad %s", arg);
- continue;
- }
- if (!strcmp(arg, "--keep-true-parents")) {
- grafts_replace_parents = 0;
- continue;
- }
- usage(pack_usage);
+ if (rev_list_unpacked) {
+ use_internal_rev_list = 1;
+ rp_av[rp_ac++] = "--unpacked";
+ }
+ if (rev_list_all) {
+ use_internal_rev_list = 1;
+ rp_av[rp_ac++] = "--all";
+ }
+ if (rev_list_reflog) {
+ use_internal_rev_list = 1;
+ rp_av[rp_ac++] = "--reflog";
}
/* Traditionally "pack-objects [options] base extra" failed;
@@ -2497,12 +2449,25 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
* walker.
*/
- if (!pack_to_stdout)
- base_name = argv[i++];
-
- if (pack_to_stdout != !base_name)
- usage(pack_usage);
+ if (!pack_to_stdout) {
+ if (!argc)
+ die("base name required if --stdout is not given");
+ base_name = argv[0];
+ argc--;
+ }
+ if (argc)
+ die("base name or --stdout are mutually exclusive");
+ if (!reuse_object)
+ reuse_delta = 0;
+ if (pack_compression_level == -1)
+ pack_compression_level = Z_DEFAULT_COMPRESSION;
+ else if (pack_compression_level < 0 || pack_compression_level > Z_BEST_COMPRESSION)
+ die("bad pack compression level %d", pack_compression_level);
+#ifdef NO_PTHREADS
+ if (delta_search_threads != 1)
+ warning("no threads support, ignoring %s", arg);
+#endif
if (!pack_to_stdout && !pack_size_limit)
pack_size_limit = pack_size_limit_cfg;
if (pack_to_stdout && pack_size_limit)
--
1.7.8.36.g69ee2
^ permalink raw reply related
* Disable merge conflicts on on non-conflicting changes to subsequent lines
From: Marcin Wiśnicki @ 2012-01-31 10:55 UTC (permalink / raw)
To: git
If two subsequent lines are changed in different branches, merging
will result in conflict event if there isn't any.
Invoking merge tool will mark file as resolved automatically.
Is there a way to disable this behavior ? I want the merge to happen
automatically if possible.
^ permalink raw reply
* Re: Bug: "git checkout -b" should be allowed in empty repo
From: demerphq @ 2012-01-31 10:11 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Michael Haggerty, Junio C Hamano, git
In-Reply-To: <4F27BBED.9080902@viscovery.net>
On 31 January 2012 11:01, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Am 1/31/2012 9:57, schrieb Michael Haggerty:
>> No, the idea is to avoid special casing by making 0{40} into a real (but
>> empty) revision.
>
> But then why not just have git init perform the equivalent of
>
> c=$(echo "Start" | git commit-tree $empty_tree_sha1) &&
> git update-ref refs/heads/master $c
>
> People who dislike an empty initial commit can always use "git commit
> --amend" for the first "real" commit.
Because it would then violate a system invariant that all commits are
descendants of the root commit.
You can model a git commit graph as a pathway through multidimensional
space of all possible commit trees. From that perspective it makes
sense that every pathway starts at the origin point of the
multidimensional space, which is conceptually the same as the proposed
root commit.
Anyway, I am not saying it should change, just that from some point of
views it makes a lot of sense.
Yves
--
perl -Mre=debug -e "/just|another|perl|hacker/"
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox