Git development
 help / color / mirror / Atom feed
* Re: "trying out" gitolite with minimum impact on a system
From: Sitaram Chamarty @ 2011-10-12  7:52 UTC (permalink / raw)
  To: Philippe Vaucher; +Cc: Git Mailing List
In-Reply-To: <CAGK7Mr6cnP6QQwGswWwQYiGR2_BUjMPz+VsygQXb0Voehm+akg@mail.gmail.com>

On Wed, Oct 12, 2011 at 11:45 AM, Philippe Vaucher
<philippe.vaucher@gmail.com> wrote:
>> After that, entirely within that user, you have an admin user and six
>> normal users, to do with as you please.  You emulate different users
>> simply by using a different username in the URL, like "git clone
>> u1:reponame" versus "git clone u2:reponame".
>
> Hum, except if I missed something the classic way to use gitolite is
> to always clone using the same user (git@host:repository.git), and the
> "real" identification is done by the ssh keys (which means that
> contrary to plain ssh you lose the ability to have two users with the
> same ssh key, which should never happen anyway).
>
> But maybe you're refering to an alternate authentification mechanism
> within gitolite I'm unaware of.

you should read "entirely within that user" as "entirely within that
new, possibly throw-away, Unix userid you created".  This is meant to
fit with the subject line's "minimum impact on a system" phrase.

regards

sitaram

^ permalink raw reply

* Re: [PATCH] Improving performance with pthreads in refresh_index().
From: Simon Klinkert @ 2011-10-12  7:21 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: git
In-Reply-To: <4E942D76.7030908@drmicha.warpmail.net>


Okay. It seems like my idea was already implemented by Linus. I wasn't aware of that fact.

Anyway, I've learned a bit more about git's bowels. Thanks a lot.

Simon

On 11.10.2011, at 13:50, Michael J Gruber wrote:

> klinkert@webgods.de venit, vidit, dixit 11.10.2011 11:32:
>> Git performs for every file in a repository at least one (with a cold cache)
>> lstat(). In larger repositories operations like git status take a
>> long time. In case your local repository is located on a remote server
>> (e. g. mounted via nfs) it ends up in an *incredible* slow git.
>> 
>> With this patch you're able to determine a number of threads (maxthreads)
>> in your config file to run these tons of lstats in threads. There
>> won't be created any pthreads if you haven't set maxthreads. In my
>> test cases a git status with this patch performs enormously faster (over
>> two minutes before and approximately 25 seconds now). Of course, it
>> has a positive impact on other git commands, too.
> 
> Can you specify under which circumstances one should get a speedup? Our
> NFS isn't slow enough... but on a dead slow sshfs work tree I get the
> following for "git status -s":
> 
> maxthreads: 0, preloadindex: false, time: 14.73
> maxthreads: 1, preloadindex: false, time: 14.25
> maxthreads: 2, preloadindex: false, time: 13.32
> maxthreads: 3, preloadindex: false, time: 12.40
> maxthreads: 4, preloadindex: false, time: 12.65
> maxthreads: 5, preloadindex: false, time: 12.16
> maxthreads: 8, preloadindex: false, time: 12.32
> maxthreads: 10, preloadindex: false, time: 11.98
> maxthreads: 15, preloadindex: false, time: 12.31
> maxthreads: 20, preloadindex: false, time: 12.00
> maxthreads: 0, preloadindex: true, time: 12.17
> maxthreads: 1, preloadindex: true, time: 11.98
> maxthreads: 2, preloadindex: true, time: 12.21
> maxthreads: 3, preloadindex: true, time: 11.99
> maxthreads: 4, preloadindex: true, time: 12.14
> maxthreads: 5, preloadindex: true, time: 12.21
> maxthreads: 8, preloadindex: true, time: 12.14
> maxthreads: 10, preloadindex: true, time: 12.08
> maxthreads: 15, preloadindex: true, time: 12.16
> maxthreads: 20, preloadindex: true, time: 11.96
> 
> So it seams it gives me what preloadindex does, which is not much.
> 
> Note: I'm not saying the patch is bad. I'm just wondering whether that
> is expected.
> 
> Michael
> P.S.: It's actually sshfs with ssh to an NFSv3 client (server restricts
> exports) :(

^ permalink raw reply

* Re: "trying out" gitolite with minimum impact on a system
From: Philippe Vaucher @ 2011-10-12  6:15 UTC (permalink / raw)
  To: Sitaram Chamarty; +Cc: Git Mailing List
In-Reply-To: <CAMK1S_g5CnP+vrE71cqMgcjpj8ocE+wdtA2vPjeaXGCRNt25Dw@mail.gmail.com>

> After that, entirely within that user, you have an admin user and six
> normal users, to do with as you please.  You emulate different users
> simply by using a different username in the URL, like "git clone
> u1:reponame" versus "git clone u2:reponame".

Hum, except if I missed something the classic way to use gitolite is
to always clone using the same user (git@host:repository.git), and the
"real" identification is done by the ssh keys (which means that
contrary to plain ssh you lose the ability to have two users with the
same ssh key, which should never happen anyway).

But maybe you're refering to an alternate authentification mechanism
within gitolite I'm unaware of.

Philippe

^ permalink raw reply

* Re: [RFC/PATCH]: reverse bisect v 2.0
From: Junio C Hamano @ 2011-10-12  4:57 UTC (permalink / raw)
  To: Andrew Ardill
  Cc: Christian Couder, Jeff King, Michal Vyskocil, git,
	Sverre Rabbelier, Johannes Sixt
In-Reply-To: <CAH5451kUf=vPfgOOusmJjfbiyueX9VByJLzZ9WbyqLd0z78wxA@mail.gmail.com>

Andrew Ardill <andrew.ardill@gmail.com> writes:

> Examples.
> Search for a feature add:
>    $ git bisect start --introduced='feature: git frotz says xyzzy' v0.99 master
>    Bisecting: 171 revisions left to test after this (roughly 8 steps)
>    $ ... build and then test ...
>    $ git bisect tested
>    Does this snapshot have 'feature: git frotz says xyzzy' [y/n]? yes
> # already added, look backwards
>
> Search for a feature regression:
>    $ git bisect start --removed='feature: git frotz says xyzzy' v0.99 master
>    Bisecting: 171 revisions left to test after this (roughly 8 steps)
>    $ ... build and then test ...
>    $ git bisect tested
>    Does this snapshot have 'feature: git frotz says xyzzy' [y/n]? yes
> # not removed yet, look forwards

With an obvious addition of non-interactive short-cut subcommands "git
bisect yes" and "git bisect no", I think --removed= is a much better
wording than --used-to= I suggested in the discussion.

I however am still worried about the flipping of the mapping between
<good,bad> and <yes,no> this design requires. What are we going to do to
the labels of low-level machinery (i.e $GIT_DIR/refs/bisect/bad and
$GIT/refs/bisect/good)? They appear in "bisect visualize" and I was hoping
that it would be simpler in the code if we do not have to change them in
such a way that depends on this introduced/removed switch, and that was
the reason why I was trying to see if we can solve this without the
switchable mapping between <good,bad> and <yes,no>.

More specifically, I was hoping that we can rename "good" to "old" and
"bad" to "new" unconditionally and be done with it. We would ask the user
"What did the code used to do in the olden days?" and "Does this version
behave the same as it used to?". The possible answers the user can give
are "git bisect old" (it behaves the same as the older versions) and "git
bisect new" (it behaves the same as the newer versions). Then we do not
have to worry about having to flip the meaning of <yes> and <no> at the UI
level.

^ permalink raw reply

* Re: Re* [PATCH v3 19/22] resolve_ref(): emit warnings for improperly-formatted references
From: Jeff King @ 2011-10-12  4:50 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Michael Haggerty, git, cmn, A Large Angry SCM, Daniel Barkalow,
	Sverre Rabbelier
In-Reply-To: <7v39eyddoc.fsf@alter.siamese.dyndns.org>

On Tue, Oct 11, 2011 at 09:41:39PM -0700, Junio C Hamano wrote:

> Whether we remove the warning or not, I think it would be an improvement
> not to look at random files directly underneath $GIT_DIR/. I am not sure
> how we can be confident that we caught everything, though.
> 
> In other words, is shorten-unambiguous-refs the last one that needs
> fixing? How would we know for certain?

My assumption was that the set of rules is defined by
ref_rev_parse_rules, so grepping for functions that access that would be
sufficient.

It looks like there is also ref_fetch_rules, which serves a similar
purpose. Uses of that would also need to be audited.

I _think_ that should be enough for arbitrary lookup. I'm sure other
callsites directly say things like 'git_path("MERGE_HEAD")', but that's
not a problem. They would be doing so with a well-defined top-level
refname. This is really just about the ref_rev_parse_rules lookups.

-Peff

^ permalink raw reply

* Re: [RFC] teach --edit to git rebase
From: Junio C Hamano @ 2011-10-12  4:44 UTC (permalink / raw)
  To: Jean Privat; +Cc: git
In-Reply-To: <CAMQw0oOBEjW3yS2+wcktXDuEuUiHKjfbK2qDzKvBOiwxo7Zkow@mail.gmail.com>

Jean Privat <jean.privat@gmail.com> writes:

> Yes I know "show me the code" but because I am lazy I prefer ask 1-if
> the proposal makes sense, and 2-if the following way of doing it makes
> sense.

No, no, and no we do not necessarily hate talking to lazy people but only
as long as what they propose makes some sense.

The only thing you can do with this new option is "update one commit
buried in the history, and rebase everything that build on top of it", as
far as I can tell. It feels a shame to waste the generic word "--edit" for
such a narrow option.

At the UI level, "git commit --amend HEAD~4" might be a more natural way
to invoke such an operation, I would think.

^ permalink raw reply

* Re: Re* [PATCH v3 19/22] resolve_ref(): emit warnings for improperly-formatted references
From: Junio C Hamano @ 2011-10-12  4:41 UTC (permalink / raw)
  To: Jeff King
  Cc: Michael Haggerty, git, cmn, A Large Angry SCM, Daniel Barkalow,
	Sverre Rabbelier
In-Reply-To: <20111012021128.GA32149@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> At any rate, I think the changes should be all or nothing. If the
> warning goes away, fine. But if the warning stays, and dwim_ref is going
> to have special rules for looking in the top-level $GIT_DIR, then things
> like shorten_unambiguous_ref need to respect those rules, or we've just
> created a new bug.

Whether we remove the warning or not, I think it would be an improvement
not to look at random files directly underneath $GIT_DIR/. I am not sure
how we can be confident that we caught everything, though.

In other words, is shorten-unambiguous-refs the last one that needs
fixing? How would we know for certain?

Also I tend to think Michael's "only warn in refs/" is probably not the
right solution. When a caller asks to resolve_ref(MERGE_HEAD), one of
these things can be true:

 - A file $GIT_DIR/MERGE_HEAD does not exist; this is not inherently an
   error unless we were supposed to be in the middle of a conflicted
   merge.

 - A file $GIT_DIR/MERGE_HEAD exists, and records a correct 40-hexadecimal
   get_sha1_hex() can grok. This is perfectly normal.

 - A file $GIT_DIR/MERGE_HEAD exists, but get_sha1_hex() does not grok
   it. Michael warns against this twice, and I think it is a wrong thing
   to pass this unnoticed.

Once we tighten all the "too loose accesses to $GIT_DIR/$random_filename",
we might even want to have an option to cause the caller to die() in the
error case, and the logic is the same for refs under $GIT_DIR/refs/, not
just the ref-like-things directly under $GIT_DIR.

A regular ref, can also appear in $GIT_DIR/packed-refs, but a corruption
of an entry in the file will be caught when the file is read and outside
the scope of this discussion, I think.

^ permalink raw reply

* Re: Re* [PATCH v3 19/22] resolve_ref(): emit warnings for improperly-formatted references
From: Michael Haggerty @ 2011-10-12  2:56 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jeff King, git, cmn, A Large Angry SCM, Daniel Barkalow,
	Sverre Rabbelier
In-Reply-To: <7vehyjcckp.fsf@alter.siamese.dyndns.org>

On 10/12/2011 01:50 AM, Junio C Hamano wrote:
> It starts sounding like that the ill-thought-out warning should be ripped
> out regardless of what other things we do.

ISTM that the warning is proving its worth already by illuminating some
questionable practices (treating any file in $GIT_DIR as a potential
reference) :-)

However, it might be that fixing 100% of the questionable practices is
too much work.  In that case, I suggest that the warning not be ripped
out altogether, but rather that the warning only be emitted for invalid
references whose names start with "refs/".

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

^ permalink raw reply

* Re: Re* [PATCH v3 19/22] resolve_ref(): emit warnings for improperly-formatted references
From: Jeff King @ 2011-10-12  2:11 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Michael Haggerty, git, cmn, A Large Angry SCM, Daniel Barkalow,
	Sverre Rabbelier
In-Reply-To: <7vehyjcckp.fsf@alter.siamese.dyndns.org>

On Tue, Oct 11, 2011 at 04:50:46PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > It looks like we also use it in remote.c:count_refspec_match, but I
> > haven't figured out if that can trigger a warning or not.
> 
> It starts sounding like that the ill-thought-out warning should be ripped
> out regardless of what other things we do.

Maybe. I think it is not the warning that is wrong, but that it is
exposing a slightly hack-ish part of git (that we consider things like
$GIT_DIR/config as possible refs, and just silently reject them because
they happen not to have the right format).

On the other hand, it has been working fine that way for years, so maybe
it is not worth changing now.

At any rate, I think the changes should be all or nothing. If the
warning goes away, fine. But if the warning stays, and dwim_ref is going
to have special rules for looking in the top-level $GIT_DIR, then things
like shorten_unambiguous_ref need to respect those rules, or we've just
created a new bug.

-Peff

^ permalink raw reply

* Re: Re* [PATCH v3 19/22] resolve_ref(): emit warnings for improperly-formatted references
From: Junio C Hamano @ 2011-10-11 23:50 UTC (permalink / raw)
  To: Jeff King
  Cc: Michael Haggerty, git, cmn, A Large Angry SCM, Daniel Barkalow,
	Sverre Rabbelier
In-Reply-To: <20111011230749.GA29785@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> It looks like we also use it in remote.c:count_refspec_match, but I
> haven't figured out if that can trigger a warning or not.

It starts sounding like that the ill-thought-out warning should be ripped
out regardless of what other things we do.

^ permalink raw reply

* Re: [PATCH] Make is_gitfile a non-static generic function
From: Junio C Hamano @ 2011-10-11 23:45 UTC (permalink / raw)
  To: Phil Hord; +Cc: git@vger.kernel.org
In-Reply-To: <4E94C8AB.3040807@cisco.com>

Phil Hord <hordp@cisco.com> writes:

> The new is_gitfile is an amalgam of similar functional checks
> from different places in the code....

> diff --git a/builtin/clone.c b/builtin/clone.c
> index 488f48e..5110399 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -120,13 +120,7 @@ static char *get_repo_path(const char *repo, int
> *is_bundle)
>  			return xstrdup(absolute_path(path));
>  		} else if (S_ISREG(st.st_mode) && st.st_size > 8) {
>  			/* Is it a "gitfile"? */
> -			char signature[8];
> -			int len, fd = open(path, O_RDONLY);
> -			if (fd < 0)
> -				continue;
> -			len = read_in_full(fd, signature, 8);
> -			close(fd);
> -			if (len != 8 || strncmp(signature, "gitdir: ", 8))
> +			if (!is_gitfile(path))
>  				continue;
>  			path = read_gitfile(path);
>  			if (path) {
> diff --git a/cache.h b/cache.h
> index 601f6f6..7a8d9f9 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -441,6 +441,7 @@ extern const char *get_git_work_tree(void);
>  extern const char *read_gitfile(const char *path);
>  extern const char *resolve_gitdir(const char *suspect);
>  extern void set_git_work_tree(const char *tree);
> +extern int is_gitfile(const char *path);
>   #define ALTERNATE_DB_ENVIRONMENT "GIT_ALTERNATE_OBJECT_DIRECTORIES"
>  diff --git a/transport.c b/transport.c
> index f3195c0..d08a826 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -859,7 +859,11 @@ static int is_local(const char *url)
>  		has_dos_drive_prefix(url);
>  }
>  -static int is_gitfile(const char *url)
> +/*
> + * See if the referenced file looks like a 'gitfile'.
> + * Does not try to determine if the referenced gitdir is actually valid.
> + */
> +int is_gitfile(const char *url)
>  {
>  	struct stat st;
>  	char buf[9];

After looking at this patch and the way the other caller in transport.c
uses it, I am more and more convinced that "is_gitfile()" is a stupid and
horrible mistake.

The caller in transport.c says "I am about to read from a regular file,
and usually I would treat it as a bundle, but I want to avoid that
codepath if that regular file is not a bundle. So I use the codepath only
when that file is not a gitfile".

It should be saying "Is it a bundle? Then I'd use the codepath to read
from the bundle" to begin with. Otherwise the code will break when we add
yet another regular file we can fetch from that is not a bundle nor a
gitfile.

I think the hand-crafted check in builtin/clone.c you removed originated
from laziness to avoid teaching read_gitfile() to read potential gitfile
silently (and signal errors by simply returning NULL). I also suspect the
codepath may become simpler if we had a way to ask "Is this a bundle?".

I think read_bundle_header() in bundle.c can be refactored to a silent
interface that allows us to ask "Is this a bundle?" question properly.

^ permalink raw reply

* Re: [PATCH] Make is_gitfile a non-static generic function
From: Junio C Hamano @ 2011-10-11 23:26 UTC (permalink / raw)
  To: Phil Hord; +Cc: git@vger.kernel.org
In-Reply-To: <4E94C8AB.3040807@cisco.com>

Phil Hord <hordp@cisco.com> writes:

> I'm not sure this function belongs in transport.c anymore, but
> I left it here to minimize conflicts.  I think a better home would
> be path.c, but maybe not.  If someone has a preference,
> please let me know.

I would think either transport.c or setup.c is more appropriate than
path.c; the last one is more of "pathname manipulation utility bag of
functions" and does not have much to do with the "Git"-ness of the path
they deal with.

I am not sure if is_gitfile() is a good name for a global function,
though.  Also I think the interface to the function should be updated so
that the caller can choose to receive the target path when the function
returns with positive answer, making "read_gitfile()" unnecessary for such
a caller.

As a matter of fact, couldn't we somehow unify these two slightly
different implementations around the same theme, making is_gitfile()
function unnecessary? As far as I can tell, the only difference between
these functions is how they fail when given a non-gitfile, and many
callers just call read_gitfile() without first asking if it is a gitfile.

^ permalink raw reply

* Re: [PATCH 0/6] Negation magic pathspec
From: Junio C Hamano @ 2011-10-11 23:17 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1318373083-13840-1-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> I'm still struggling with read_directory() rewrite so that struct
> pathspec can be used throughout git, but now realized we can at least
> enable magic for certain commands and die() on those that don't.
> This may help move magic pathspec patches forward.

I actually was thinking that teaching those that take bare "const char **"
to take "struct pathspec *" is much more important conversion before
adding more magic to those that already do take "struct pathspec *". The
ones that ask "does this path match the pathspec" would automatically
start honoring negative or quantum or whatever magic once we teach
the magic to match_pathspec(), but the ones that take "const char **" will
have no way of doing so before getting converted to "struct pathspec *"
interface.  If some parts of the system knows more magic and people start
playing with them, the lack of the support in codepaths that haven't been
converted will become real pain point.

That is not to say that these 6 patch series are wasted patches. I just
think it may be doing things in a wrong order.

^ permalink raw reply

* Re: Re* [PATCH v3 19/22] resolve_ref(): emit warnings for improperly-formatted references
From: Jeff King @ 2011-10-11 23:07 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Michael Haggerty, git, cmn, A Large Angry SCM, Daniel Barkalow,
	Sverre Rabbelier
In-Reply-To: <7v39ezffq5.fsf_-_@alter.siamese.dyndns.org>

On Tue, Oct 11, 2011 at 01:14:26PM -0700, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> >> I think we've discussed tightening it a few years ago already.
> >>
> >> HEAD, MERGE_HEAD, FETCH_HEAD, etc. all are "^[_A-Z]*$" and it may even be
> >> a good idea to insist "^[_A-Z]*HEAD$" or even "^([A-Z][A-Z]*_)?HEAD$".
> >
> > Perhaps like this? Only compile tested...
> 
> Not quite. There are at least three bugs in the patch.
> 
>  - Some subsystems use random refnames like NOTES_MERGE_PARTIAL that would
>    not match "^([A-Z][A-Z]*_)?HEAD$". The rule needs to be relaxed;
> 
>  - dwim_ref() can be fed "refs/heads/master" and is expected to dwim it to
>    the master branch.
> 
>  - These codepaths get pointer+length so that it can be told to parse only
>    the first 4 bytes in "HEAD:$path".

One more bug. :)

We also look at ref_rev_parse_rules in shorten_unambiguous_ref. So even
with your patch, I still get the warning with:

  $ git branch config
  $ git for-each-ref --format='%(refname:short)' refs/heads/

It looks like we also use it in remote.c:count_refspec_match, but I
haven't figured out if that can trigger a warning or not.

-Peff

^ permalink raw reply

* Re: Git Bug - diff in commit message.
From: Junio C Hamano @ 2011-10-11 23:00 UTC (permalink / raw)
  To: git; +Cc: Michael Witten, anikey
In-Reply-To: <7vpqj9vh87.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> In the longer term, I think "git-rebase--am.sh" should be rewritten to
> have format-patch avoid the cost of actually generating the patch text,
> and the "mailinfo" call that comes above the context shown in this patch
> should be made conditional---when using "am" for rebasing we do not really
> care anything but the commit object names, and everything else is figured
> out from the commit this codepath.

And here is a quick hack to do that using "log --cherry-pick --right-only".

 git-am.sh         |   44 ++++++++++++++++++++++++--------------------
 git-rebase--am.sh |   12 +++++++++---
 2 files changed, 33 insertions(+), 23 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index 9042432..b79ccc5 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -641,32 +641,36 @@ do
 	# by the user, or the user can tell us to do so by --resolved flag.
 	case "$resume" in
 	'')
-		git mailinfo $keep $no_inbody_headers $scissors $utf8 "$dotest/msg" "$dotest/patch" \
-			<"$dotest/$msgnum" >"$dotest/info" ||
-			stop_here $this
-
-		# skip pine's internal folder data
-		sane_grep '^Author: Mail System Internal Data$' \
-			<"$dotest"/info >/dev/null &&
-			go_next && continue
-
-		test -s "$dotest/patch" || {
-			eval_gettextln "Patch is empty.  Was it split wrong?
-If you would prefer to skip this patch, instead run \"\$cmdline --skip\".
-To restore the original branch and stop patching run \"\$cmdline --abort\"."
-			stop_here $this
-		}
 		rm -f "$dotest/original-commit" "$dotest/author-script"
-		if test -f "$dotest/rebasing" &&
+
+		if test -f "$dotest/rebasing"
+		then
 			commit=$(sed -e 's/^From \([0-9a-f]*\) .*/\1/' \
 				-e q "$dotest/$msgnum") &&
-			test "$(git cat-file -t "$commit")" = commit
-		then
+			test "$(git cat-file -t "$commit")" = commit || {
+				stop_here $this
+			}
 			git cat-file commit "$commit" |
 			sed -e '1,/^$/d' >"$dotest/msg-clean"
-			echo "$commit" > "$dotest/original-commit"
-			get_author_ident_from_commit "$commit" > "$dotest/author-script"
+			echo "$commit" >"$dotest/original-commit"
+			get_author_ident_from_commit "$commit" >"$dotest/author-script"
+			git diff-tree -p --root "$commit" >"$dotest/patch"
 		else
+			git mailinfo $keep $no_inbody_headers \
+			$scissors $utf8 "$dotest/msg" "$dotest/patch" \
+			<"$dotest/$msgnum" >"$dotest/info" ||
+			stop_here $this
+
+			# skip pine's internal folder data
+			sane_grep '^Author: Mail System Internal Data$' \
+				<"$dotest"/info >/dev/null &&
+				go_next && continue
+			test -s "$dotest/patch" || {
+				eval_gettextln "Patch is empty.  Was it split wrong?
+If you would prefer to skip this patch, instead run \"\$cmdline --skip\".
+To restore the original branch and stop patching run \"\$cmdline --abort\"."
+				stop_here $this
+			}
 			{
 				sed -n '/^Subject/ s/Subject: //p' "$dotest/info"
 				echo
diff --git a/git-rebase--am.sh b/git-rebase--am.sh

[...]

^ permalink raw reply related

* Re: Re* [PATCH v3 19/22] resolve_ref(): emit warnings for improperly-formatted references
From: Jeff King @ 2011-10-11 22:54 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Lars Hjemli, Michael Haggerty, git, cmn, A Large Angry SCM,
	Daniel Barkalow, Sverre Rabbelier
In-Reply-To: <7vpqi3dxkr.fsf@alter.siamese.dyndns.org>

On Tue, Oct 11, 2011 at 02:31:48PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > But in the code, it is spelled RENAMED-REF (with a dash). And as far as
> > I can tell, does not actually create a reflog. And it's not documented
> > anywhere, so I suspect nobody is using it. Maybe it is worth switching
> > that name.
> 
> Or even better get rid of it?

Fine by me. I'm not sure anyone is even aware that it exists. Just to
double-check, I grepped the list archives, and the biggest mention of it
was its presence causing a weird bug:

  http://thread.gmane.org/gmane.comp.version-control.git/143737

Googling turns up only confusion, nobody actually using it or
recommending that it be used.

OTOH, it does actually serialize branch renames, since we take a lock on
it. Maybe that's important. Cc'ing Lars. Certainly renaming it would be
the conservative choice.

-Peff

PS I mentioned above that it does not actually create a reflog. Digging
in the code more, I think it is capable of it, but my git.git clone had
RENAMED-REF, but no reflog. I would have thought core.logAllRefUpdates
would turn it on, but maybe there is a funny interaction with things not
in refs/.

^ permalink raw reply

* [PATCH] Make is_gitfile a non-static generic function
From: Phil Hord @ 2011-10-11 22:52 UTC (permalink / raw)
  To: git@vger.kernel.org, Phil Hord
In-Reply-To: <4E94C70E.3080003@cisco.com>

The new is_gitfile is an amalgam of similar functional checks
from different places in the code.  All these places do
slightly different checks on the suspect gitfile (for their
own good reasons).  But the lack of common code leads to bugs
and maintenance problems. Help move the code forward by making
is_gitfile() a common function that everyone can use for this
purpose.

Signed-off-by: Phil Hord <hordp@cisco.com>
---

I'm not sure this function belongs in transport.c anymore, but
I left it here to minimize conflicts.  I think a better home would
be path.c, but maybe not.  If someone has a preference,
please let me know.

 builtin/clone.c |    8 +-------
 cache.h         |    1 +
 transport.c     |    6 +++++-
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 488f48e..5110399 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -120,13 +120,7 @@ static char *get_repo_path(const char *repo, int
*is_bundle)
 			return xstrdup(absolute_path(path));
 		} else if (S_ISREG(st.st_mode) && st.st_size > 8) {
 			/* Is it a "gitfile"? */
-			char signature[8];
-			int len, fd = open(path, O_RDONLY);
-			if (fd < 0)
-				continue;
-			len = read_in_full(fd, signature, 8);
-			close(fd);
-			if (len != 8 || strncmp(signature, "gitdir: ", 8))
+			if (!is_gitfile(path))
 				continue;
 			path = read_gitfile(path);
 			if (path) {
diff --git a/cache.h b/cache.h
index 601f6f6..7a8d9f9 100644
--- a/cache.h
+++ b/cache.h
@@ -441,6 +441,7 @@ extern const char *get_git_work_tree(void);
 extern const char *read_gitfile(const char *path);
 extern const char *resolve_gitdir(const char *suspect);
 extern void set_git_work_tree(const char *tree);
+extern int is_gitfile(const char *path);
  #define ALTERNATE_DB_ENVIRONMENT "GIT_ALTERNATE_OBJECT_DIRECTORIES"
 diff --git a/transport.c b/transport.c
index f3195c0..d08a826 100644
--- a/transport.c
+++ b/transport.c
@@ -859,7 +859,11 @@ static int is_local(const char *url)
 		has_dos_drive_prefix(url);
 }
 -static int is_gitfile(const char *url)
+/*
+ * See if the referenced file looks like a 'gitfile'.
+ * Does not try to determine if the referenced gitdir is actually valid.
+ */
+int is_gitfile(const char *url)
 {
 	struct stat st;
 	char buf[9];
-- 
1.7.7.334.gfc143d

^ permalink raw reply related

* [PATCH 6/6] Implement negative pathspec
From: Nguyễn Thái Ngọc Duy @ 2011-10-11 22:44 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1318373083-13840-1-git-send-email-pclouds@gmail.com>


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 I really like the mnemonic ^ but it's regex. ":^Documentation" looks
 nicer than ":~Documentation". Do we plan on supporting regex in
 pathspec too?

 We should mention these magic in a less obscure document. Glossary is
 mostly for developer discussions. git-diff may be a good place
 because it's one of the two frequently used commands (the other one
 is grep) that benefit magic the most (with a short reference from
 git.txt)

 Documentation/glossary-content.txt |    8 +++---
 cache.h                            |    1 +
 dir.c                              |    2 +
 setup.c                            |    1 +
 tree-walk.c                        |   37 +++++++++++++++++++++++++++++++++--
 5 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt
index 3595b58..9a2765d 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -319,12 +319,12 @@ top `/`;;
 	The magic word `top` (mnemonic: `/`) makes the pattern match
 	from the root of the working tree, even when you are running
 	the command from inside a subdirectory.
+
+exclude `~`;;
+	The magic word `exclude` (mnemonic: `~`) excludes paths that
+	match the pattern.
 --
 +
-Currently only the slash `/` is recognized as the "magic signature",
-but it is envisioned that we will support more types of magic in later
-versions of git.
-+
 A pathspec with only a colon means "there is no pathspec". This form
 should not be combined with other pathspec.
 
diff --git a/cache.h b/cache.h
index 719d4a3..75fe589 100644
--- a/cache.h
+++ b/cache.h
@@ -541,6 +541,7 @@ extern int ie_modified(const struct index_state *, struct cache_entry *, struct
  */
 #define PATHSPEC_FROMTOP    (1<<0)
 #define PATHSPEC_NOGLOB     (1<<1)
+#define PATHSPEC_NEGATE     (1<<2)
 
 struct pathspec {
 	const char **raw; /* get_pathspec() result, not freed by free_pathspec() */
diff --git a/dir.c b/dir.c
index d38af0f..46dd35f 100644
--- a/dir.c
+++ b/dir.c
@@ -1305,6 +1305,8 @@ int parse_pathspec(struct pathspec *pathspec, const char *prefix,
 				pitem->magic |= PATHSPEC_NOGLOB;
 			else
 				pathspec->magic &= ~PATHSPEC_NOGLOB;
+			if (pitem->magic & PATHSPEC_NEGATE)
+				pathspec->magic |= PATHSPEC_NEGATE;
 			pitem++;
 			dst++;
 		}
diff --git a/setup.c b/setup.c
index b074210..42beb9b 100644
--- a/setup.c
+++ b/setup.c
@@ -111,6 +111,7 @@ static struct pathspec_magic {
 	const char *name;
 } pathspec_magic[] = {
 	{ PATHSPEC_FROMTOP, '/', "top" },
+	{ PATHSPEC_NEGATE, '~', "exclude" },
 };
 
 /*
diff --git a/tree-walk.c b/tree-walk.c
index db07fd6..936b5da 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -580,15 +580,17 @@ static int match_dir_prefix(const char *base, int baselen,
  *  - zero for no
  *  - negative for "no, and no subsequent entries will be either"
  */
-int tree_entry_interesting(const struct name_entry *entry,
-			   struct strbuf *base, int base_offset,
-			   const struct pathspec *ps)
+static int tree_entry_interesting_1(const struct name_entry *entry,
+				    struct strbuf *base, int base_offset,
+				    const struct pathspec *ps, int negative_magic)
 {
 	int i;
 	int pathlen, baselen = base->len - base_offset;
 	int never_interesting = ps->magic & PATHSPEC_NOGLOB ? -1 : 0;
+	int has_effective_pathspec = 0;
 
 	if (!ps->nr) {
+no_pathspec:
 		if (!ps->recursive || ps->max_depth == -1)
 			return 2;
 		return !!within_depth(base->buf + base_offset, baselen,
@@ -604,6 +606,12 @@ int tree_entry_interesting(const struct name_entry *entry,
 		const char *base_str = base->buf + base_offset;
 		int matchlen = item->len;
 
+		if ((!negative_magic && !(item->magic & PATHSPEC_NEGATE)) ||
+		    ( negative_magic &&  (item->magic & PATHSPEC_NEGATE)))
+			has_effective_pathspec = 1;
+		else
+			continue;
+
 		if (baselen >= matchlen) {
 			/* If it doesn't match, move along... */
 			if (!match_dir_prefix(base_str, baselen, match, matchlen))
@@ -663,5 +671,28 @@ match_wildcards:
 		if (ps->recursive && S_ISDIR(entry->mode))
 			return 1;
 	}
+
+	/* the same effect with ps->nr == 0 */
+	if (!has_effective_pathspec)
+		goto no_pathspec;
+
 	return never_interesting; /* No matches */
 }
+
+int tree_entry_interesting(const struct name_entry *entry,
+			   struct strbuf *base, int base_offset,
+			   const struct pathspec *ps)
+{
+	int next_ret, ret;
+
+	ret = tree_entry_interesting_1(entry, base, base_offset, ps, 0);
+	if (ps->magic & PATHSPEC_NEGATE) {
+		next_ret = tree_entry_interesting_1(entry, base, base_offset, ps, 1);
+		switch (next_ret) {
+		case 2: ret = -1; break;
+		case 1: ret = 0; break;
+		default: break;
+		}
+	}
+	return ret;
+}
-- 
1.7.3.1.256.g2539c.dirty

^ permalink raw reply related

* [PATCH 5/6] Convert simple init_pathspec() cases to parse_pathspec()
From: Nguyễn Thái Ngọc Duy @ 2011-10-11 22:44 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1318373083-13840-1-git-send-email-pclouds@gmail.com>

These commands can now take advantage of new pathspec magic, if both
tree_entry_interesting() and match_pathspec_depth() support them properly

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/grep.c    |    4 +---
 builtin/ls-tree.c |    2 +-
 builtin/reset.c   |    2 +-
 revision.c        |    9 +++++----
 4 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index a286692..e171a9d 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -759,7 +759,6 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	const char *show_in_pager = NULL, *default_pager = "dummy";
 	struct grep_opt opt;
 	struct object_array list = OBJECT_ARRAY_INIT;
-	const char **paths = NULL;
 	struct pathspec pathspec;
 	struct string_list path_list = STRING_LIST_INIT_NODUP;
 	int i;
@@ -1020,8 +1019,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			verify_filename(prefix, argv[j]);
 	}
 
-	paths = get_pathspec(prefix, argv + i);
-	init_pathspec(&pathspec, paths);
+	parse_pathspec(&pathspec, prefix, -1, argv + i);
 	pathspec.max_depth = opt.max_depth;
 	pathspec.recursive = 1;
 
diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index f0fa7dd..b717bb2 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -166,7 +166,7 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
 	if (get_sha1(argv[0], sha1))
 		die("Not a valid object name %s", argv[0]);
 
-	init_pathspec(&pathspec, get_pathspec(prefix, argv + 1));
+	parse_pathspec(&pathspec, prefix, -1, argv + 1);
 	for (i = 0; i < pathspec.nr; i++)
 		pathspec.items[i].magic = PATHSPEC_NOGLOB;
 	pathspec.magic |= PATHSPEC_NOGLOB;
diff --git a/builtin/reset.c b/builtin/reset.c
index 811e8e2..8126e69 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -176,7 +176,7 @@ static int read_from_tree(const char *prefix, const char **argv,
 	struct diff_options opt;
 
 	memset(&opt, 0, sizeof(opt));
-	diff_tree_setup_paths(get_pathspec(prefix, (const char **)argv), &opt);
+	parse_pathspec(&opt.pathspec, prefix, -1, argv);
 	opt.output_format = DIFF_FORMAT_CALLBACK;
 	opt.format_callback = update_index_from_diff;
 	opt.format_callback_data = &index_was_discarded;
diff --git a/revision.c b/revision.c
index 9bae329..cba32e8 100644
--- a/revision.c
+++ b/revision.c
@@ -1770,8 +1770,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 		 */
 		ALLOC_GROW(prune_data.path, prune_data.nr+1, prune_data.alloc);
 		prune_data.path[prune_data.nr++] = NULL;
-		init_pathspec(&revs->prune_data,
-			      get_pathspec(revs->prefix, prune_data.path));
+		parse_pathspec(&revs->prune_data, revs->prefix, -1, prune_data.path);
 	}
 
 	if (revs->def == NULL)
@@ -1804,12 +1803,14 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 		revs->limited = 1;
 
 	if (revs->prune_data.nr) {
-		diff_tree_setup_paths(revs->prune_data.raw, &revs->pruning);
+		/* Careful, we share a lot of pointers here, do not free 1st arg */
+		memcpy(&revs->pruning.pathspec, &revs->prune_data, sizeof(struct pathspec));
 		/* Can't prune commits with rename following: the paths change.. */
 		if (!DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES))
 			revs->prune = 1;
 		if (!revs->full_diff)
-			diff_tree_setup_paths(revs->prune_data.raw, &revs->diffopt);
+			/* Careful, we share a lot of pointers here, do not free 1st arg */
+			memcpy(&revs->diffopt.pathspec, &revs->prune_data, sizeof(struct pathspec));
 	}
 	if (revs->combine_merges)
 		revs->ignore_merges = 0;
-- 
1.7.3.1.256.g2539c.dirty

^ permalink raw reply related

* [PATCH 4/6] Implement parse_pathspec()
From: Nguyễn Thái Ngọc Duy @ 2011-10-11 22:44 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1318373083-13840-1-git-send-email-pclouds@gmail.com>

This function is the same as get_pathspec() except that it produces
struct pathspec directly.

no_prefix code path is necessary because init_pathspec() can be called
without get_pathspec_old() in "diff --no-index" case. Without this
exception, prefix_path() will be eventually called and die because
there is not worktree.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 cache.h |    5 ++++
 dir.c   |   82 ++++++++++++++++++++++++++++++++++++++++++++++----------------
 setup.c |    4 +-
 3 files changed, 68 insertions(+), 23 deletions(-)

diff --git a/cache.h b/cache.h
index 17df06f..719d4a3 100644
--- a/cache.h
+++ b/cache.h
@@ -443,6 +443,9 @@ extern void set_git_work_tree(const char *tree);
 
 #define ALTERNATE_DB_ENVIRONMENT "GIT_ALTERNATE_OBJECT_DIRECTORIES"
 
+struct pathspec_item;
+extern const char *prefix_pathspec(struct pathspec_item *item, const char *prefix,
+				   int prefixlen, const char *elt);
 extern const char **get_pathspec(const char *prefix, const char **pathspec);
 extern const char *pathspec_prefix(const char *prefix, const char **pathspec);
 extern void setup_work_tree(void);
@@ -554,6 +557,8 @@ struct pathspec {
 };
 
 extern int init_pathspec(struct pathspec *, const char **);
+extern int parse_pathspec(struct pathspec *pathspec, const char *prefix,
+			  int argc, const char **argv);
 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 6c82615..d38af0f 100644
--- a/dir.c
+++ b/dir.c
@@ -18,6 +18,8 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, in
 	int check_only, const struct path_simplify *simplify);
 static int get_dtype(struct dirent *de, const char *path, int len);
 
+static const char *no_prefix = "We do not want outside repository check.";
+
 /* helper string functions with support for the ignore_case flag */
 int strcmp_icase(const char *a, const char *b)
 {
@@ -1252,34 +1254,62 @@ static int pathspec_item_cmp(const void *a_, const void *b_)
 	return strcmp(a->match, b->match);
 }
 
-int init_pathspec(struct pathspec *pathspec, const char **paths)
+extern const char *prefix_pathspec(struct pathspec_item *item, const char *prefix,
+				   int prefixlen, const char *elt);
+int parse_pathspec(struct pathspec *pathspec, const char *prefix,
+		   int argc, const char **argv)
 {
-	const char **p = paths;
-	int i;
+	struct pathspec_item *pitem;
+	const char **dst;
+	int prefixlen;
 
 	memset(pathspec, 0, sizeof(*pathspec));
-	if (!p)
-		return 0;
-	while (*p)
-		p++;
-	pathspec->raw = paths;
-	pathspec->nr = p - paths;
-	pathspec->magic = PATHSPEC_NOGLOB;
-	if (!pathspec->nr)
+
+	if (argc == -1) {
+		argc = 0;
+		for (dst = argv; *dst; dst++)
+			argc++;
+	}
+
+	if ((!prefix || prefix == no_prefix) && !argc)
 		return 0;
 
-	pathspec->items = xmalloc(sizeof(struct pathspec_item)*pathspec->nr);
-	memset(pathspec->items, 0, sizeof(struct pathspec_item)*pathspec->nr);
-	for (i = 0; i < pathspec->nr; i++) {
-		struct pathspec_item *item = pathspec->items+i;
-		const char *path = paths[i];
+	if (!argc) {
+		static const char *spec[2];
+		spec[0] = prefix;
+		spec[1] = NULL;
+		init_pathspec(pathspec, spec);
+		pathspec->items[0].plain_len = pathspec->items[0].len;
+		return 0;
+	}
 
-		item->match = path;
-		item->len = strlen(path);
-		if (no_wildcard(path))
-			item->magic |= PATHSPEC_NOGLOB;
+	prefixlen = prefix && prefix != no_prefix ? strlen(prefix) : 0;
+	pathspec->nr = argc; /* be optimistic, lower it later if necessary */
+	pathspec->items = xmalloc(sizeof(struct pathspec_item) * argc);
+	pathspec->raw = argv;
+	pathspec->magic = PATHSPEC_NOGLOB;
+	pitem = pathspec->items;
+	dst = argv;
+
+	while (*argv) {
+		if (prefix == no_prefix) {
+			memset(pitem, 0, sizeof(*pitem));
+			pitem->match = *argv;
+			pitem->len = strlen(pitem->match);
+		}
+		else
+			prefix_pathspec(pitem, prefix, prefixlen, *argv);
+		*dst = *argv++;
+		if (pitem->match && pitem->len) {
+			if (no_wildcard(pitem->match + pitem->plain_len))
+				pitem->magic |= PATHSPEC_NOGLOB;
+			else
+				pathspec->magic &= ~PATHSPEC_NOGLOB;
+			pitem++;
+			dst++;
+		}
 		else
-			pathspec->magic &= ~PATHSPEC_NOGLOB;
+			pathspec->nr--;
 	}
 
 	qsort(pathspec->items, pathspec->nr,
@@ -1288,8 +1318,18 @@ int init_pathspec(struct pathspec *pathspec, const char **paths)
 	return 0;
 }
 
+int init_pathspec(struct pathspec *pathspec, const char **paths)
+{
+	const char **p = paths;
+	while (p && *p)
+		p++;
+	return parse_pathspec(pathspec, no_prefix, p - paths, paths);
+}
+
 void free_pathspec(struct pathspec *pathspec)
 {
+	/* memory leak: pathspec_item->match likely be xstrdup'd by
+	   prefix_pathspec */
 	free(pathspec->items);
 	pathspec->items = NULL;
 }
diff --git a/setup.c b/setup.c
index 8f1c2c0..b074210 100644
--- a/setup.c
+++ b/setup.c
@@ -126,8 +126,8 @@ static struct pathspec_magic {
  * the prefix part must always match literally, and a single stupid
  * string cannot express such a case.
  */
-static const char *prefix_pathspec(struct pathspec_item *item, const char *prefix,
-				   int prefixlen, const char *elt)
+const char *prefix_pathspec(struct pathspec_item *item, const char *prefix,
+			    int prefixlen, const char *elt)
 {
 	unsigned magic = 0;
 	const char *copyfrom = elt;
-- 
1.7.3.1.256.g2539c.dirty

^ permalink raw reply related

* [PATCH 3/6] Convert prefix_pathspec() to produce struct pathspec_item
From: Nguyễn Thái Ngọc Duy @ 2011-10-11 22:44 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1318373083-13840-1-git-send-email-pclouds@gmail.com>

New field plain_len is used to mark the first part of 'match' where no
magic can be applied. It's not used though. tree_entry_interesting()
should learn to utilize it.

Now we can show get_pathspec() what magic a pathspec has. Make sure
only certain magic is accepted because more will come in future and
not all of them can be converted to plain string like
PATHSPEC_FROMTOP.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 match_pathspec_depth() should also check for unsupported magic..

 cache.h |    1 +
 setup.c |   22 +++++++++++++++++-----
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/cache.h b/cache.h
index fe46f1e..17df06f 100644
--- a/cache.h
+++ b/cache.h
@@ -548,6 +548,7 @@ struct pathspec {
 	struct pathspec_item {
 		const char *match;
 		int len;
+		int plain_len;	/* match[0..plain_len-1] does not have any kind of magic*/
 		unsigned magic;
 	} *items;
 };
diff --git a/setup.c b/setup.c
index 35db910..8f1c2c0 100644
--- a/setup.c
+++ b/setup.c
@@ -126,7 +126,8 @@ static struct pathspec_magic {
  * the prefix part must always match literally, and a single stupid
  * string cannot express such a case.
  */
-static const char *prefix_pathspec(const char *prefix, int prefixlen, const char *elt)
+static const char *prefix_pathspec(struct pathspec_item *item, const char *prefix,
+				   int prefixlen, const char *elt)
 {
 	unsigned magic = 0;
 	const char *copyfrom = elt;
@@ -181,10 +182,17 @@ static const char *prefix_pathspec(const char *prefix, int prefixlen, const char
 			copyfrom++;
 	}
 
+	memset(item, 0, sizeof(*item));
+	item->magic = magic;
+
 	if (magic & PATHSPEC_FROMTOP)
-		return xstrdup(copyfrom);
-	else
-		return prefix_path(prefix, prefixlen, copyfrom);
+		item->match = xstrdup(copyfrom);
+	else {
+		item->match = prefix_path(prefix, prefixlen, copyfrom);
+		item->plain_len = prefixlen;
+	}
+	item->len = strlen(item->match);
+	return 0;
 }
 
 const char **get_pathspec(const char *prefix, const char **pathspec)
@@ -208,7 +216,11 @@ const char **get_pathspec(const char *prefix, const char **pathspec)
 	dst = pathspec;
 	prefixlen = prefix ? strlen(prefix) : 0;
 	while (*src) {
-		*(dst++) = prefix_pathspec(prefix, prefixlen, *src);
+		struct pathspec_item item;
+		prefix_pathspec(&item, prefix, prefixlen, *src);
+		if (item.magic & ~PATHSPEC_FROMTOP)
+			die("Unsupported magic pathspec %s", *src);
+		*(dst++) = item.match;
 		src++;
 	}
 	*dst = NULL;
-- 
1.7.3.1.256.g2539c.dirty

^ permalink raw reply related

* [PATCH 2/6] Replace has_wildcard with PATHSPEC_NOGLOB
From: Nguyễn Thái Ngọc Duy @ 2011-10-11 22:44 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1318373083-13840-1-git-send-email-pclouds@gmail.com>

Note though "noglob" magic is not implemented yet, only its definition
for internal use.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/ls-files.c |    2 +-
 builtin/ls-tree.c  |    4 ++--
 cache.h            |   22 ++++++++++++++++++++--
 dir.c              |   11 +++++++----
 setup.c            |   17 -----------------
 tree-walk.c        |    7 +++----
 6 files changed, 33 insertions(+), 30 deletions(-)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index e8a800d..e687157 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -326,7 +326,7 @@ void overlay_tree_on_cache(const char *tree_name, const char *prefix)
 		matchbuf[0] = prefix;
 		matchbuf[1] = NULL;
 		init_pathspec(&pathspec, matchbuf);
-		pathspec.items[0].use_wildcard = 0;
+		pathspec.items[0].magic = PATHSPEC_NOGLOB;
 	} else
 		init_pathspec(&pathspec, NULL);
 	if (read_tree(tree, 1, &pathspec))
diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index 6b666e1..f0fa7dd 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -168,8 +168,8 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
 
 	init_pathspec(&pathspec, get_pathspec(prefix, argv + 1));
 	for (i = 0; i < pathspec.nr; i++)
-		pathspec.items[i].use_wildcard = 0;
-	pathspec.has_wildcard = 0;
+		pathspec.items[i].magic = PATHSPEC_NOGLOB;
+	pathspec.magic |= PATHSPEC_NOGLOB;
 	tree = parse_tree_indirect(sha1);
 	if (!tree)
 		die("not a tree object");
diff --git a/cache.h b/cache.h
index 82e12c8..fe46f1e 100644
--- a/cache.h
+++ b/cache.h
@@ -521,16 +521,34 @@ extern int index_name_is_other(const struct index_state *, const char *, int);
 extern int ie_match_stat(const struct index_state *, struct cache_entry *, struct stat *, unsigned int);
 extern int ie_modified(const struct index_state *, struct cache_entry *, struct stat *, unsigned int);
 
+/*
+ * Magic pathspec
+ *
+ * NEEDSWORK: These need to be moved to dir.h or even to a new
+ * pathspec.h when we restructure get_pathspec() users to use the
+ * "struct pathspec" interface.
+ *
+ * Possible future magic semantics include stuff like:
+ *
+ *	{ PATHSPEC_NOGLOB, '!', "noglob" },
+ *	{ PATHSPEC_ICASE, '\0', "icase" },
+ *	{ PATHSPEC_RECURSIVE, '*', "recursive" },
+ *	{ PATHSPEC_REGEXP, '\0', "regexp" },
+ *
+ */
+#define PATHSPEC_FROMTOP    (1<<0)
+#define PATHSPEC_NOGLOB     (1<<1)
+
 struct pathspec {
 	const char **raw; /* get_pathspec() result, not freed by free_pathspec() */
 	int nr;
-	unsigned int has_wildcard:1;
+	unsigned magic;
 	unsigned int recursive:1;
 	int max_depth;
 	struct pathspec_item {
 		const char *match;
 		int len;
-		unsigned int use_wildcard:1;
+		unsigned magic;
 	} *items;
 };
 
diff --git a/dir.c b/dir.c
index 08281d2..6c82615 100644
--- a/dir.c
+++ b/dir.c
@@ -230,7 +230,7 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix,
 			return MATCHED_RECURSIVELY;
 	}
 
-	if (item->use_wildcard && !fnmatch(match, name, 0))
+	if (!(item->magic & PATHSPEC_NOGLOB) && !fnmatch(match, name, 0))
 		return MATCHED_FNMATCH;
 
 	return 0;
@@ -1264,19 +1264,22 @@ int init_pathspec(struct pathspec *pathspec, const char **paths)
 		p++;
 	pathspec->raw = paths;
 	pathspec->nr = p - paths;
+	pathspec->magic = PATHSPEC_NOGLOB;
 	if (!pathspec->nr)
 		return 0;
 
 	pathspec->items = xmalloc(sizeof(struct pathspec_item)*pathspec->nr);
+	memset(pathspec->items, 0, sizeof(struct pathspec_item)*pathspec->nr);
 	for (i = 0; i < pathspec->nr; i++) {
 		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;
+		if (no_wildcard(path))
+			item->magic |= PATHSPEC_NOGLOB;
+		else
+			pathspec->magic &= ~PATHSPEC_NOGLOB;
 	}
 
 	qsort(pathspec->items, pathspec->nr,
diff --git a/setup.c b/setup.c
index 08f605b..35db910 100644
--- a/setup.c
+++ b/setup.c
@@ -105,23 +105,6 @@ void verify_non_filename(const char *prefix, const char *arg)
 	    "Use '--' to separate filenames from revisions", arg);
 }
 
-/*
- * Magic pathspec
- *
- * NEEDSWORK: These need to be moved to dir.h or even to a new
- * pathspec.h when we restructure get_pathspec() users to use the
- * "struct pathspec" interface.
- *
- * Possible future magic semantics include stuff like:
- *
- *	{ PATHSPEC_NOGLOB, '!', "noglob" },
- *	{ PATHSPEC_ICASE, '\0', "icase" },
- *	{ PATHSPEC_RECURSIVE, '*', "recursive" },
- *	{ PATHSPEC_REGEXP, '\0', "regexp" },
- *
- */
-#define PATHSPEC_FROMTOP    (1<<0)
-
 static struct pathspec_magic {
 	unsigned bit;
 	char mnemonic; /* this cannot be ':'! */
diff --git a/tree-walk.c b/tree-walk.c
index 808bb55..db07fd6 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -586,7 +586,7 @@ int tree_entry_interesting(const struct name_entry *entry,
 {
 	int i;
 	int pathlen, baselen = base->len - base_offset;
-	int never_interesting = ps->has_wildcard ? 0 : -1;
+	int never_interesting = ps->magic & PATHSPEC_NOGLOB ? -1 : 0;
 
 	if (!ps->nr) {
 		if (!ps->recursive || ps->max_depth == -1)
@@ -625,7 +625,7 @@ int tree_entry_interesting(const struct name_entry *entry,
 					&never_interesting))
 				return 1;
 
-			if (ps->items[i].use_wildcard) {
+			if (!(ps->items[i].magic & PATHSPEC_NOGLOB)) {
 				if (!fnmatch(match + baselen, entry->path, 0))
 					return 1;
 
@@ -636,12 +636,11 @@ int tree_entry_interesting(const struct name_entry *entry,
 				if (ps->recursive && S_ISDIR(entry->mode))
 					return 1;
 			}
-
 			continue;
 		}
 
 match_wildcards:
-		if (!ps->items[i].use_wildcard)
+		if (ps->items[i].magic & PATHSPEC_NOGLOB)
 			continue;
 
 		/*
-- 
1.7.3.1.256.g2539c.dirty

^ permalink raw reply related

* [PATCH 1/6] Recognize magic pathspec as filenames
From: Nguyễn Thái Ngọc Duy @ 2011-10-11 22:44 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1318373083-13840-1-git-send-email-pclouds@gmail.com>


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 .. so that "git log :/" works, not so sure this is correct though

 setup.c |   16 +++++++---------
 1 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/setup.c b/setup.c
index 27c1d47..08f605b 100644
--- a/setup.c
+++ b/setup.c
@@ -58,15 +58,8 @@ static void NORETURN die_verify_filename(const char *prefix, const char *arg)
 	unsigned char sha1[20];
 	unsigned mode;
 
-	/*
-	 * Saying "'(icase)foo' does not exist in the index" when the
-	 * user gave us ":(icase)foo" is just stupid.  A magic pathspec
-	 * begins with a colon and is followed by a non-alnum; do not
-	 * let get_sha1_with_mode_1(only_to_die=1) to even trigger.
-	 */
-	if (!(arg[0] == ':' && !isalnum(arg[1])))
-		/* try a detailed diagnostic ... */
-		get_sha1_with_mode_1(arg, sha1, &mode, 1, prefix);
+	/* try a detailed diagnostic ... */
+	get_sha1_with_mode_1(arg, sha1, &mode, 1, prefix);
 
 	/* ... or fall back the most general message. */
 	die("ambiguous argument '%s': unknown revision or path not in the working tree.\n"
@@ -85,6 +78,11 @@ void verify_filename(const char *prefix, const char *arg)
 {
 	if (*arg == '-')
 		die("bad flag '%s' used after filename", arg);
+
+	/* If it's magic pathspec, just assume it's file name */
+	if (arg[0] == ':' && is_pathspec_magic(arg[1]))
+		return;
+
 	if (check_filename(prefix, arg))
 		return;
 	die_verify_filename(prefix, arg);
-- 
1.7.3.1.256.g2539c.dirty

^ permalink raw reply related

* [PATCH 0/6] Negation magic pathspec
From: Nguyễn Thái Ngọc Duy @ 2011-10-11 22:44 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

After the last round toying with .gitignore mechanism as a way to
exclude paths, I have finally got back to the negative pathspec.

I'm still struggling with read_directory() rewrite so that struct
pathspec can be used throughout git, but now realized we can at least
enable magic for certain commands and die() on those that don't.
This may help move magic pathspec patches forward.

The nice thing about this series is that negative pathspec patch is
small and simple, much less headache to review than the previous
version (and as a consequence, not as powerful).

So here it is to gather comments whether we should go this way. Very
much WIP, I have not even run "make test".

Nguyễn Thái Ngọc Duy (6):
  Recognize magic pathspec as filenames
  Replace has_wildcard with PATHSPEC_NOGLOB
  Convert prefix_pathspec() to produce struct pathspec_item
  Implement parse_pathspec()
  Convert simple init_pathspec() cases to parse_pathspec()
  Implement negative pathspec

 Documentation/glossary-content.txt |    8 ++--
 builtin/grep.c                     |    4 +-
 builtin/ls-files.c                 |    2 +-
 builtin/ls-tree.c                  |    6 +-
 builtin/reset.c                    |    2 +-
 cache.h                            |   29 +++++++++++-
 dir.c                              |   85 +++++++++++++++++++++++++++--------
 revision.c                         |    9 ++--
 setup.c                            |   56 +++++++++++-------------
 tree-walk.c                        |   44 ++++++++++++++++---
 10 files changed, 169 insertions(+), 76 deletions(-)

-- 
1.7.3.1.256.g2539c.dirty

^ permalink raw reply

* Re: [RFC] teach --edit to git rebase
From: Jean Privat @ 2011-10-11 22:36 UTC (permalink / raw)
  To: git
In-Reply-To: <CAMQw0oOBEjW3yS2+wcktXDuEuUiHKjfbK2qDzKvBOiwxo7Zkow@mail.gmail.com>

> An other alternative is to use a simple git alias for myself (and do
> not bother the git community) but I do not know how to automatize the
> command 'git rebase --interactive' I suppose I need more complex
> infrastructure in the existing 'git rebase' (Maybe I did not look
> enough and there is a way to do it with a git alias).

Hum. I just found that I can do something like :

[alias]
        edit=!GIT_EDITOR='sed -i -e 1s/pick/edit/ --' git rebase -i
-no-autosquash $1^

The main thing that bothers me is that after the rebase, the head is
detached (why?)

The other thing is that the error messages for invalid usages are far
for perfect

$ git edit toto
fatal: Needed a single revision
invalid upstream toto^

$ git edit --root toto
error: unknown option `root^'
usage: git rebase [-i] [options] [--onto <newbase>] [<upstream>] [<branch>]
[...]

-Jean

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox