Git development
 help / color / mirror / Atom feed
* Re: git send-email should not allow 'y' for in-reply-to
From: Hilco Wijbenga @ 2013-01-11 18:43 UTC (permalink / raw)
  To: Jeff King; +Cc: Eric Blake, git, libvir-list
In-Reply-To: <20130111164730.GA7921@sigill.intra.peff.net>

On 11 January 2013 08:47, Jeff King <peff@peff.net> wrote:
> On Fri, Jan 11, 2013 at 09:39:06AM -0700, Eric Blake wrote:
>
>> > Please don't answer "y" when git send email shows the following prompt:
>> >
>> > "Message-ID to be used as In-Reply-To for the first email?"
>> >
>> > you should respond with a message ID there. Unfortunately we have a
>> > growing thread that contains submissions with this mistake.

<snip/>

>   People answer 'y' to "Who should the emails appear to be from?"  and
>   'n' to "Message-ID to be used as In-Reply-To for the first email?"
>   for some unknown reason.  While it is possible that your local
>   username really is "y" and you are sending the mail to your local
>   colleagues, it is possible, and some might even say it is likely,
>   that it is a user error.

I have never used Git's email support so this doesn't affect me one
way or another but it seems that checking the results is fixing the
symptoms, not the problem? I apologize if this was already discussed
but I couldn't find such a discussion.

I was wondering if it might be a better idea to change the wording of
the questions if they have proven so confusing? The first time (just
now) that I read "Message-ID to be used as In-Reply-To for the first
email?", it clearly seemed like a yes/no question to me. :-)

How about "What Message-ID to use as In-Reply-To for the first email?"
or "Provide the Message-ID to use as In-Reply-To for the first
email:". I'm a little surprised that "Who should the emails appear to
be from?" would be interpreted as a yes/no question but we could
rephrase that similarly as "Provide the name of the email sender:" (I
don't really like this particular version but you get the idea).

^ permalink raw reply

* Re: parsecvs has been salvaged
From: Bart Massey @ 2013-01-11 18:25 UTC (permalink / raw)
  To: Eric S. Raymond; +Cc: git, Keith Packard
In-Reply-To: <20130111112151.AFF924047B@snark.thyrsus.com>

Very cool! I'm glad you got it doing what you wanted; I'll be
interested to see how parsecvs compares in quality and performance to
cvs2git and cvsps. --Bart

On Fri, Jan 11, 2013 at 3:21 AM, Eric S. Raymond <esr@thyrsus.com> wrote:
> Since Heiko Voigt and others were concerned about this, I report that
> I have successfully salvaged the parsecvs code. I now have it emitting
> a correct-looking fast-import stream for my main test repository.
>
> I'm not ready to ship it yet because there are several features I
> think it ought to have before I do.  An -R option like cvsps's;
> correct interpretation of a third timezone field as in cvsps; and,
> most significantly, I want to make sure it emits warnings for important
> error and problem conditions like unresolvable tags and absence of
> commitids.
>
> But these are all relatively minor issues. It is likely I will be able
> to ship early next week, at which point I will add support for
> parsecvs as a third engine in new cvsimport.
>
> This next step in the larger program will be factoring out the cvsps
> test suite and applying it to all three of cvsps, cvs2git, and
> parsecvs so I can compare results.
> --
>                 <a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
>
> Americans have the right and advantage of being armed - unlike the citizens
> of other countries whose governments are afraid to trust the people with arms.
>         -- James Madison, The Federalist Papers

^ permalink raw reply

* Re: [PATCH v2 03/21] Export parse_pathspec() and convert some get_pathspec() calls
From: Matt Kraai @ 2013-01-11 17:56 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano
In-Reply-To: <1357903275-16804-4-git-send-email-pclouds@gmail.com>

On Fri, Jan 11, 2013 at 06:20:57PM +0700, Nguyễn Thái Ngọc Duy wrote:
> +#define PATHSPEC_FROMTOP    (1<<0)

The previous commit introduces a use of this macro in get_pathspec.
Should this be defined by that commit instead?

> @@ -266,9 +266,9 @@ static int pathspec_item_cmp(const void *a_, const void *b_)
>   * Given command line arguments and a prefix, convert the input to
>   * pathspec. die() if any magic other than ones in magic_mask.
>   */
> -static void parse_pathspec(struct pathspec *pathspec,
> -			   unsigned magic_mask, unsigned flags,
> -			   const char *prefix, const char **argv)
> +void parse_pathspec(struct pathspec *pathspec,
> +		    unsigned magic_mask, unsigned flags,

The prototype for this function uses just "magic" instead of
"magic_mask".  Should they be consistent?

-- 
Matt

^ permalink raw reply

* Re: git send-email should not allow 'y' for in-reply-to
From: Eric Blake @ 2013-01-11 17:51 UTC (permalink / raw)
  To: Jeff King; +Cc: git, libvir-list
In-Reply-To: <20130111164730.GA7921@sigill.intra.peff.net>

[-- Attachment #1: Type: text/plain, Size: 1390 bytes --]

On 01/11/2013 09:47 AM, Jeff King wrote:
> On Fri, Jan 11, 2013 at 09:39:06AM -0700, Eric Blake wrote:
> 
>>> Please don't answer "y" when git send email shows the following prompt:
>>>

>>
>> Anyone willing to patch upstream 'git send-email' to reject a simple 'y'

> What version of git? Commit 51bbccf is in v1.7.12.1 and higher, and
> says:
> 
>   $ git show 51bbccf
>   commit 51bbccfd1b4a9e2807413022c56ab05c835164fb
>   Author: Junio C Hamano <gitster@pobox.com>
>   Date:   Tue Aug 14 15:15:53 2012 -0700
> 
>   send-email: validate & reconfirm interactive responses
> 
>   People answer 'y' to "Who should the emails appear to be from?"  and
>   'n' to "Message-ID to be used as In-Reply-To for the first email?"
>   for some unknown reason.  While it is possible that your local
>   username really is "y" and you are sending the mail to your local
>   colleagues, it is possible, and some might even say it is likely,
>   that it is a user error.

Awesome!  Already implemented!  In the case that sparked this particular
email, the culprit was using 1.7.3.4; earlier this month, a separate
culprit to the same libvirt mailing list was using 1.7.11.7.

I was right about it needing to take a few months to percolate to the
actual users.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

^ permalink raw reply

* Re: [PATCH 09/19] reset.c: replace switch by if-else
From: Junio C Hamano @ 2013-01-11 17:12 UTC (permalink / raw)
  To: Martin von Zweigbergk; +Cc: git
In-Reply-To: <CANiSa6gz-DBv+2gUDPdhgmeYdHg3-OVO80a7NvdLn4vYRyKEnA@mail.gmail.com>

Martin von Zweigbergk <martinvonz@gmail.com> writes:

>> Justification?
>
> Clairvoyance ...

;-)

^ permalink raw reply

* Re: git send-email should not allow 'y' for in-reply-to
From: Jeff King @ 2013-01-11 16:47 UTC (permalink / raw)
  To: Eric Blake; +Cc: git, libvir-list
In-Reply-To: <50F0402A.1000108@redhat.com>

On Fri, Jan 11, 2013 at 09:39:06AM -0700, Eric Blake wrote:

> > Please don't answer "y" when git send email shows the following prompt:
> > 
> > "Message-ID to be used as In-Reply-To for the first email?"
> > 
> > you should respond with a message ID there. Unfortunately we have a
> > growing thread that contains submissions with this mistake.
> 
> Anyone willing to patch upstream 'git send-email' to reject a simple 'y'
> rather than blindly sending a bad messageID for the in-reply-to field,
> to help future users avoid this mistake?  Obviously, it won't help until
> the patch eventually percolates into distros, so it would be a few more
> months before we see the benefits, but down the road it will prevent
> confusing threads.

What version of git? Commit 51bbccf is in v1.7.12.1 and higher, and
says:

  $ git show 51bbccf
  commit 51bbccfd1b4a9e2807413022c56ab05c835164fb
  Author: Junio C Hamano <gitster@pobox.com>
  Date:   Tue Aug 14 15:15:53 2012 -0700

  send-email: validate & reconfirm interactive responses

  People answer 'y' to "Who should the emails appear to be from?"  and
  'n' to "Message-ID to be used as In-Reply-To for the first email?"
  for some unknown reason.  While it is possible that your local
  username really is "y" and you are sending the mail to your local
  colleagues, it is possible, and some might even say it is likely,
  that it is a user error.

  Fortunately, our interactive prompter already has input validation
  mechanism built-in.  Enhance it so that we can optionally reconfirm
  and allow the user to pass an input that does not validate, and
  "softly" require input to the sender, in-reply-to, and recipient to
  contain "@" and "." in this order, which would catch most cases of
  mistakes.

-Peff

^ permalink raw reply

* Re: [PATCH] git-completion.bash: Silence not a valid object errors
From: Junio C Hamano @ 2013-01-11 16:45 UTC (permalink / raw)
  To: Dylan Smith; +Cc: git
In-Reply-To: <alpine.DEB.2.02.1301110304220.26739@antec>

Dylan Smith <dylan.ah.smith@gmail.com> writes:

> Trying to complete the command
>
>   git show master:./file
>
> would cause a "Not a valid object name" error to be output on standard
> error. Silence the error so it won't appear on the command line.
>
> Signed-off-by: Dylan Smith <dylan.ah.smith@gmail.com>
> ---

Looks obviously correct.  Thanks.

>  contrib/completion/git-completion.bash |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 0b77eb1..d4c7bfe 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -397,7 +397,7 @@ __git_complete_revlist_file ()
>  		*)   pfx="$ref:$pfx" ;;
>  		esac
>  
> -		__gitcomp_nl "$(git --git-dir="$(__gitdir)" ls-tree "$ls" \
> +		__gitcomp_nl "$(git --git-dir="$(__gitdir)" ls-tree "$ls" 2>/dev/null \
>  				| sed '/^100... blob /{
>  				           s,^.*	,,
>  				           s,$, ,

^ permalink raw reply

* Re: missing objects -- prevention
From: Jeff King @ 2013-01-11 16:42 UTC (permalink / raw)
  To: Sitaram Chamarty; +Cc: Git Mailing List
In-Reply-To: <CAMK1S_jpofLRO02XTYryOP98g7rnrJXs7Mh2zvi=SoVUAs0dUw@mail.gmail.com>

On Fri, Jan 11, 2013 at 04:40:38PM +0530, Sitaram Chamarty wrote:

> I find a lot of info on how to recover from and/or repair a repo that
> has missing (or corrupted) objects.
> 
> What I need is info on common reasons (other than disk errors -- we've
> checked for those) for such errors to occur, any preventive measures
> we can take, and so on.

I don't think any race can cause corruption of the object or packfiles
because of the way they are written. At GitHub, every case of file-level
corruption we've seen has been a filesystem issue.

So I think the main thing systemic/race issue to worry about is missing
objects. And since git only deletes objects during a prune (assuming you
are using git-gc or "repack -A" so that repack cannot drop objects), I
think prune is the only thing to watch out for.

The --expire time saves us from the obvious races where you write object
X but have not yet referenced it, and a simultaneous prune wants to
delete it. However, it's possible that you have an old object that is
unreferenced, but would become referenced as a result of an in-progress
operation. For example, commit X is unreferenced and ready to be pruned,
you build commit Y on top of it, but before you write the ref, git-prune
removes X.

The server-side version of that would happen via receive-pack, and is
even more unlikely, because X would have to be referenced initially for
us to advertise it. So it's something like:

  1. The repo has a ref R pointing at commit X.

  2. A user starts a push to another ref, Q, of commit Y that builds on
     X. Git advertises ref R, so the sender knows they do not need to
     send X, but only Y. The user then proceeds to send the packfile
     (which might take a very long time).

  3. Meanwhile, another user deletes ref R. X becomes unreferenced.

  4. After step 3 but before step 2 has finished, somebody runs prune
     (this might sound unlikely, but if you kick off a "gc" job after
     each push, or after N pushes, it's not so unlikely).  It sees that
     X is unreferenced, and it may very well be older than the --expire
     setting. Prune deletes X.

  5. The packfile in (2) arrives, and receive-pack attempts to update
     the refs.

So it's even a bit more unlikely than the local case, because
receive-pack would not otherwise build on dangling objects. You have
to race steps (2) and (3) just to create the situation.

Then we have an extra protection in the form of
check_everything_connected, which receive-pack runs before writing the
refs into place. So if step 4 happens while the packfile is being sent
(which is the most likely case, since it is the longest stretch of
receive-pack's time), we would still catch it there and reject the push
(annoying to the user, but the repo remains consistent).

However, that's not foolproof. We might hit step 4 after we've checked
that everything is connected but right before we write the ref. In which
case we drop X, which has just become referenced, and we have a missing
object.

So I think it's possible. But I have never actually seen it in practice,
and come up with this scenario only by brainstorming "what could go
wrong" scenarios.

This could be mitigated if there was a "proposed refs" storage.
Receive-pack would write a note saying "consider Y for pruning purposes,
but it's not really referenced yet", check connectivity for Y against
the current refs, and then eventually write Y to its real ref (or reject
it if there are problems). Prune would either run before the "proposed"
note is written, which would mean it deletes X, but the connectivity
check fails. Or it would run after, in which case it would leave X
alone.

> For example, can *any* type of network error or race condition cause
> this?  (Say, can one push writes an object, then fails an update
> check, and a later push succeeds and races against a gc that removes
> the unreachable object?)  Or... the repo is pretty large -- about 6-7
> GB, so could size cause a race that would not show up on a smaller
> repo?

The above is the only open issue I know about. I don't think it is
dependent on repo size, but the window is widened for a really large
push, because rev-list takes longer to run. It does not widen if you
have receive.fsckobjects set, because that happens before we do the
connectivity check (and the connectivity check is run in a sub-process,
so the race timer starts when we exec rev-list, which may open and mmap
packfiles or otherwise cache the presence of X in memory).

> Anything else I can watch out for or caution the team about?

That's the only open issue I know about for missing objects.

There is a race with simultaneously deleting and packing refs. It
doesn't cause object db corruption, but it will cause refs to "rewind"
back to their packed versions. I have seen that one in practice (though
relatively rare). I fixed it in b3f1280, which is not yet in any
released version.

> The symptom is usually a disk space crunch caused by tmp_pack_* files
> left around by auto-gc.  Presumably the missing objects failed the gc
> and so it left the files around, and they eventually accumulate enough
> to cause disk full errors.  (If a gc ever succeeded, I suspect these
> files would go away, but that requires manual intervention).

Gc shouldn't be leaving around tmp_pack_* unless it is actually dying
during the pack-objects phase. In my experience, stale tmp_pack_*
objects are more likely a sign of a failed push (e.g., the client hangs
up in the middle, or fsck rejects the pack). We have historically left
them in place to facilitate analysis of the failure.

At GitHub, we've taken to just cleaning them up aggressively (I think
after an hour), though I am tempted to put in an optional signal/atexit
handler to clean them up when index-pack fails. The forensics are
occasionally interesting (e.g., finding weird fsck problems), but large
pushes can waste a lot of disk space in the interim.

-Peff

^ permalink raw reply

* git send-email should not allow 'y' for in-reply-to
From: Eric Blake @ 2013-01-11 16:39 UTC (permalink / raw)
  To: git; +Cc: libvir-list
In-Reply-To: <50EFD066.60501@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 949 bytes --]

[raising this UI wart to the git list]

On 01/11/2013 01:42 AM, Peter Krempa wrote:
> On 01/11/13 07:31, Chunyan Liu wrote:
>> This patch series is to...
[snip]

> 
> Please don't answer "y" when git send email shows the following prompt:
> 
> "Message-ID to be used as In-Reply-To for the first email?"
> 
> you should respond with a message ID there. Unfortunately we have a
> growing thread that contains submissions with this mistake.

Anyone willing to patch upstream 'git send-email' to reject a simple 'y'
rather than blindly sending a bad messageID for the in-reply-to field,
to help future users avoid this mistake?  Obviously, it won't help until
the patch eventually percolates into distros, so it would be a few more
months before we see the benefits, but down the road it will prevent
confusing threads.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

^ permalink raw reply

* Re: [PATCH] cvsimport: rewrite to use cvsps 3.x to fix major bugs
From: Junio C Hamano @ 2013-01-11 16:31 UTC (permalink / raw)
  To: Eric S. Raymond; +Cc: git
In-Reply-To: <1357875152-19899-1-git-send-email-gitster@pobox.com>

> From: "Eric S. Raymond" <esr@thyrsus.com>
> ...
> diff --git a/git-cvsimport.perl b/git-cvsimport-fallback.perl
> similarity index 98%
> rename from git-cvsimport.perl
> rename to git-cvsimport-fallback.perl
> index 0a31ebd..4bc0717 100755
> --- a/git-cvsimport.perl
> +++ b/git-cvsimport-fallback.perl
> @@ -1,4 +1,8 @@
>  #!/usr/bin/perl
> +# This code became obsolete in January 2013, and is retained only as a
> +# fallback from git-cvsimport.py for users who have only cvsps-2.x.
> +# It (and the code in cvsimport.py that calls it) should be removed
> +# once the 3.x version has had a reasonable time to propagate.
>  
>  # This tool is copyright (c) 2005, Matthias Urlichs.
>  # It is released under the Gnu Public License, version 2.
> @@ -27,6 +31,10 @@
>  use POSIX qw(strftime tzset dup2 ENOENT);
>  use IPC::Open2;
>  
> +print(STDERR "You do not appear to have cvsps 3.x.\n");
> +print(STDERR "Falling back to unmaintained Perl cvsimport for cvsps 2.x.\n");
> +print(STDERR "Upgrade your cvsps for best results.\n");

I think the prevalent style in this script is to write "print"
without parentheses:

	print STDERR "msg\n";

> diff --git a/git-cvsimport.py b/git-cvsimport.py
> new file mode 100755
> index 0000000..129471e
> --- /dev/null
> +++ b/git-cvsimport.py
> @@ -0,0 +1,354 @@
> +#!/usr/bin/env python
> +#
> +# Import CVS history into git
> +#
> +# Intended to be a near-workalike of Matthias Urlichs's Perl implementation.
> ...
> +class cvsps:
> +    "Method class for cvsps back end."
> +    def __init__(self):
> +        self.opts = ""
> +        self.revmap = None
> +    def set_repo(self, val):
> +        "Set the repository root option."
> +        if not val.startswith(":"):
> +            if not val.startswith(os.sep):
> +                val = os.path.abspath(val)
> +            val = ":local:" + val
> +        self.opts += " --root '%s'" % val

This looks lazy and unsafe quoting.  Is there anything that makes
sure repository path does not contain a single quote?

> +    def set_authormap(self, val):
> +        "Set the author-map file."
> +        self.opts += " -A '%s'" % val
> +    def set_fuzz(self, val):
> +        "Set the commit-similarity window."
> +        self.opts += " -z %s" % val
> +    def set_nokeywords(self):
> +        "Suppress CVS keyword expansion."
> +        self.opts += " -k"
> +    def add_opts(self, val):
> +        "Add options to the engine command line."
> +        self.opts += " " + val

... especially for callers of this method.

The same comment applies to many uses of "val" in the method
implementations of this class and the cvs2git class.

> +    def command(self):
> +        "Emit the command implied by all previous options."
> +        return "(cvs2git --username=git-cvsimport --quiet --quiet --blobfile={0} --dumpfile={1} {2} {3} && cat {0} {1} && rm {0} {1})".format(tempfile.mkstemp()[1], tempfile.mkstemp()[1], self.opts, self.modulepath)

Could we do something better with this overlong source line?

> +if __name__ == '__main__':
> ...
> +    for (opt, val) in options:
> +        if opt == '-v':
> +            verbose += 1
> +        elif opt == '-b':
> +            bare = True
> +        elif opt == '-e':
> +            for cls in (cvsps, cvs2git):
> +                if cls.__name__ == val:
> +                    backend = cls()
> +                    break
> +            else:
> +                sys.stderr.write("git cvsimport: unknown engine %s.\n" % val)
> +                sys.exit(1)
> +        elif opt == '-d':
> +            backend.set_repo(val)
> +        elif opt == '-C':
> +            outdir = val
> +        elif opt == '-r':
> +            remotize = True
> +        elif opt == '-o':
> +            sys.stderr.write("git cvsimport: -o is no longer supported.\n")
> +            sys.exit(1)

Isn't this a regression?

> +        elif opt == '-i':
> +            import_only = True
> +        elif opt == '-k':
> +            backend.set_nokeywords()
> +        elif opt == '-u':
> +            underscore_to_dot = True
> +        elif opt == '-s':
> +            slashsubst = val
> +        elif opt == '-p':
> +            backend.add_opts(val.replace(",", " "))
> +        elif opt == '-z':
> ...
> +        elif opt == '-P':
> +            backend = filesource(val)
> +            sys.exit(1)

???

> +        elif opt in ('-m', '-M'):
> +            sys.stderr.write("git cvsimport: -m and -M are no longer supported: use reposurgeon instead.\n")
> +            sys.exit(1)

I wonder if it is better to ignore these options with a warning but
still let the command continue; cvsps-3.x was supposed to get merges
right without the help of these ad-hoc options, no?

Otherwise it looks like a regression to me.

> +        elif opt == '-S':
> +            backend.set_exclusion(val)
> +        elif opt == '-a':
> +            sys.stderr.write("git cvsimport: -a is no longer supported.\n")
> +            sys.exit(1)
> +        elif opt == '-L':
> +            sys.stderr.write("git cvsimport: -L is no longer supported.\n")
> +            sys.exit(1)
> +        elif opt == '-A':
> +            authormap = os.path.abspath(val)
> +        elif opt == '-R':
> +            revisionmap = True
> +        else:
> +            print """\
> +git cvsimport [-A <author-conv-file>] [-C <git_repository>] [-b] [-d <CVSROOT>]
> +     [-e engine] [-h] [-i] [-k] [-p <options-for-cvsps>] [-P <source-file>]
> +     [-r <remote>] [-R] [-s <subst>] [-S <regex>] [-u] [-v] [-z <fuzz>]
> +     [<CVS_module>]
> +"""
> +    def metadata(fn, outdir='.'):
> +        if bare:
> +            return os.path.join(outdir, fn)
> +        else:
> +            return os.path.join(outdir, ".git", fn)
> +    # Ugly fallback code for people with only cvsps-2.x
> +    # Added January 2013 - should be removed after a decent interval.
> +    if backend.__class__.__name__ == "cvsps":
> +        try:
> +            subprocess.check_output("cvsps -V 2>/dev/null", shell=True)
> +        except subprocess.CalledProcessError as e:
> +            if e.returncode == 1:
> +                sys.stderr.write("cvsimport: falling back to old version...\n")
> +                sys.exit(os.system("git-cvsimport-fallback " + " ".join(sys.argv[1:])))
> +            else:
> +                sys.stderr.write("cvsimport: cannot execute cvsps.\n")
> +                sys.exit(1)

Having the code to die when it sees options the rewritten version
does not yet support before it calls the fallback makes the fallback
much less effective, no?

> +    # Real mainline code begins here
> +    try:
> +        if outdir:
> +            try:
> +                # If the output directory does not exist, create it
> +                # and initialize it as a git repository.
> +                os.mkdir(outdir)
> +                do_or_die("git init --quiet " + outdir)

Did anything made sure outdir is without $IFS chars up to this
point?

Not very impressed (yet).  The advertised "fix major bugs" sounds
more like "trade major bugs with different ones with a couple of
feature removals" at this point.

^ permalink raw reply

* Re: [PATCH v2 0/2] improve-wincred-compatibility
From: Erik Faye-Lund @ 2013-01-11 16:20 UTC (permalink / raw)
  To: blees; +Cc: git, msysgit, Jeff King
In-Reply-To: <50EEAF9A.6020302@gmail.com>

On Thu, Jan 10, 2013 at 1:10 PM, Karsten Blees <karsten.blees@gmail.com> wrote:
> Changes since initial version (see attached diff for details):
> - split in two patches
> - removed unused variables
> - improved the dll error message
> - changed ?: to if else
> - added comments
>
> Also available here:
> https://github.com/kblees/git/tree/kb/improve-wincred-compatibility-v2
> git pull git://github.com/kblees/git.git kb/improve-wincred-compatibility-v2
>
> Karsten Blees (2):
>   wincred: accept CRLF on stdin to simplify console usage
>   wincred: improve compatibility with windows versions
>
>  .../credential/wincred/git-credential-wincred.c    | 206 ++++++++-------------
>  1 file changed, 75 insertions(+), 131 deletions(-)
>
>

Wonderful!

Acked-by: Erik Faye-Lund <kusmabite@gmail.com>

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

^ permalink raw reply

* Re: What's cooking in git.git (Jan 2013, #04; Wed, 9)
From: Junio C Hamano @ 2013-01-11 16:07 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: git
In-Reply-To: <CACsJy8CZ9YuzRW6-dh6293EQJAhCDL9wsMdb=NSxMRq060wPbA@mail.gmail.com>

Duy Nguyen <pclouds@gmail.com> writes:

> On Thu, Jan 10, 2013 at 4:04 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Here are the topics that have been cooking.  Commits prefixed with
>> '-' are only in 'pu' (proposed updates) while commits prefixed with
>> '+' are in 'next'.
>
> I see you start to use no-change commits as a way to keep notes in
> 'pu'. Isn't git notes designed for that?

It is not designed for this.

You could (ab)use notes for that purpose, but at the semantic level,
I do not think notes is a good match.

The branches 'pu' and its subset 'jch' (designed to be slightly
ahead of 'next', and this is what I regularly do my Git work with)
are rebuilt on top of 'master' in every integration cycle (like 2 to
3 times a day).  The insn to rebuild the latter currently looks like
this (familiarity with the Documentation/howto/maintain-git.txt
document in 'pu' would help understanding this section):

    $ cat Meta/redo-jch.sh
    Meta/Reintegrate <<\EOF
    jc/format-patch-reroll
    nd/retire-fnmatch
    ...
    jk/maint-fast-import-doc-reorder
    nd/upload-pack-shallow-must-be-commit
    ap/log-mailmap
    ### match next
    jc/doc-maintainer
    mk/complete-tcsh
    pe/doc-email-env-is-trumped-by-config
    as/check-ignore
    EOF

The Reintegrate machinery merges the topic listed on each line, and
the "### match next" marks the point _between_ the two merges to
remind me that merging everything before this line to 'master' must
match what the 'next' branch has.  The markers on 'pu' are similar.

You can emulate that semantics by attaching a note to the merge
commit that merges ap/log-mailmap topic, but that is unwieldy, and
more importantly, it does not correctly express the meaning of the
marker.  I often reorder lines before that "### match next" marker
between integration rounds, to float topics that are more ready than
others near the top, to make sure that they do not depend on other
topics and cause mismerges when merged to 'master' without others.
The next integration cycle may have ap/log-mailmap earlier, e.g.

    $ cat Meta/redo-jch.sh
    Meta/Reintegrate <<\EOF
    nd/upload-pack-shallow-must-be-commit
    ap/log-mailmap
    jc/format-patch-reroll
    ...
    jk/maint-fast-import-doc-reorder
    nd/retire-fnmatch
    ### match next
    jc/doc-maintainer
    mk/complete-tcsh
    ...

and having a note on the previous merge of ap/log-mailmap does not
help expressing the semantics correctly.  It is not like I cease to
declare the merge of ap/log-mailmap ought to match 'next' and now
declare the merge of nk/retire-fnmatch ought to match 'next'.

It's much clearer to have the marker as a separate entity, as
opposed to emulating it with an attribute of a merge commit that
happens to be adjacent to what I really want to mark (i.e. a place
*between* two merge commits).

Another way you could use notes to emulate this is to give each
merge that comes before the marker line a note that says "this merge
is before the marker line".  But it still is more work to present
the result as "here is a sequence, then a marker, and then another
sequence, then another marker...".  In short, notes may be able to
emulate it but it is not the best tool for the job.

It is not like I'll be running format-patch and rebasing these
throw-away integration branches, so no-change commits are the best
way to express what I want to in the history.

^ permalink raw reply

* Re: [PATCH] git-completion.bash: Silence not a valid object errors
From: Jeff King @ 2013-01-11 15:27 UTC (permalink / raw)
  To: Dylan Smith; +Cc: git
In-Reply-To: <alpine.DEB.2.02.1301110304220.26739@antec>

On Fri, Jan 11, 2013 at 03:06:22AM -0500, Dylan Smith wrote:

> Trying to complete the command
> 
>   git show master:./file
> 
> would cause a "Not a valid object name" error to be output on standard
> error. Silence the error so it won't appear on the command line.
> 
> Signed-off-by: Dylan Smith <dylan.ah.smith@gmail.com>
> ---

Thanks, I've been annoyed by this, too. The fix looks obviously correct.

-Peff

^ permalink raw reply

* [PATCH v2 22/21] Convert the last use of match_pathspec() and remove it
From: Nguyễn Thái Ngọc Duy @ 2013-01-11 14:26 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1357903275-16804-22-git-send-email-pclouds@gmail.com>


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 I was digging the t7400.38 failure and found that add.c is the last
 place that uses match_pathspec(). This is the first yay moment for me
 since 'struct pathspec' was introduced. I wanted to remove this
 function for so long!

 Good luck resolving conflicts by the way ;-)

 builtin/add.c | 47 ++++++++++++-----------------
 dir.c         | 97 -----------------------------------------------------------
 dir.h         |  1 -
 3 files changed, 20 insertions(+), 125 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index 6b6a72e..1235eb9 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -99,7 +99,7 @@ int add_files_to_cache(const char *prefix,
 	return !!data.add_errors;
 }
 
-static void fill_pathspec_matches(const char **pathspec, char *seen, int specs)
+static void fill_pathspec_matches(struct pathspec *pathspec, char *seen)
 {
 	int num_unmatched = 0, i;
 
@@ -109,49 +109,43 @@ static void fill_pathspec_matches(const char **pathspec, char *seen, int specs)
 	 * mistakenly think that the user gave a pathspec that did not match
 	 * anything.
 	 */
-	for (i = 0; i < specs; i++)
+	for (i = 0; i < pathspec->nr; i++)
 		if (!seen[i])
 			num_unmatched++;
 	if (!num_unmatched)
 		return;
 	for (i = 0; i < active_nr; i++) {
 		struct cache_entry *ce = active_cache[i];
-		match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, seen);
+		match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, seen);
 	}
 }
 
-static char *find_used_pathspec(const char **pathspec)
+static char *find_used_pathspec(struct pathspec *pathspec)
 {
 	char *seen;
-	int i;
-
-	for (i = 0; pathspec[i];  i++)
-		; /* just counting */
-	seen = xcalloc(i, 1);
-	fill_pathspec_matches(pathspec, seen, i);
+	seen = xcalloc(pathspec->nr, 1);
+	fill_pathspec_matches(pathspec, seen);
 	return seen;
 }
 
-static char *prune_directory(struct dir_struct *dir, const char **pathspec, int prefix)
+static char *prune_directory(struct dir_struct *dir, struct pathspec *pathspec, int prefix)
 {
 	char *seen;
-	int i, specs;
+	int i;
 	struct dir_entry **src, **dst;
 
-	for (specs = 0; pathspec[specs];  specs++)
-		/* nothing */;
-	seen = xcalloc(specs, 1);
+	seen = xcalloc(pathspec->nr, 1);
 
 	src = dst = dir->entries;
 	i = dir->nr;
 	while (--i >= 0) {
 		struct dir_entry *entry = *src++;
-		if (match_pathspec(pathspec, entry->name, entry->len,
-				   prefix, seen))
+		if (match_pathspec_depth(pathspec, entry->name, entry->len,
+					 prefix, seen))
 			*dst++ = entry;
 	}
 	dir->nr = dst - dir->entries;
-	fill_pathspec_matches(pathspec, seen, specs);
+	fill_pathspec_matches(pathspec, seen);
 	return seen;
 }
 
@@ -406,7 +400,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 		/* This picks up the paths that are not tracked */
 		baselen = fill_directory(&dir, &pathspec);
 		if (pathspec.nr)
-			seen = prune_directory(&dir, pathspec._raw, baselen);
+			seen = prune_directory(&dir, &pathspec, baselen);
 	}
 
 	if (refresh_only) {
@@ -420,17 +414,16 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 
 		path_exclude_check_init(&check, &dir);
 		if (!seen)
-			seen = find_used_pathspec(pathspec._raw);
-		for (i = 0; pathspec._raw[i]; i++) {
-			if (!seen[i] && pathspec._raw[i][0]
-			    && !file_exists(pathspec._raw[i])) {
+			seen = find_used_pathspec(&pathspec);
+		for (i = 0; i < pathspec.nr; i++) {
+			const char *path = pathspec.items[i].match;
+			if (!seen[i] && !file_exists(path)) {
 				if (ignore_missing) {
 					int dtype = DT_UNKNOWN;
-					if (path_excluded(&check, pathspec._raw[i], -1, &dtype))
-						dir_add_ignored(&dir, pathspec._raw[i], strlen(pathspec._raw[i]));
+					if (path_excluded(&check, path, -1, &dtype))
+						dir_add_ignored(&dir, path, pathspec.items[i].len);
 				} else
-					die(_("pathspec '%s' did not match any files"),
-					    pathspec._raw[i]);
+					die(_("pathspec '%s' did not match any files"), path);
 			}
 		}
 		free(seen);
diff --git a/dir.c b/dir.c
index aad180b..44ff9bb 100644
--- a/dir.c
+++ b/dir.c
@@ -114,103 +114,6 @@ int within_depth(const char *name, int namelen,
  *
  * It returns 0 when there is no match.
  */
-static int match_one(const char *match, const char *name, int namelen)
-{
-	int matchlen;
-
-	/* If the match was just the prefix, we matched */
-	if (!*match)
-		return MATCHED_RECURSIVELY;
-
-	if (ignore_case) {
-		for (;;) {
-			unsigned char c1 = tolower(*match);
-			unsigned char c2 = tolower(*name);
-			if (c1 == '\0' || is_glob_special(c1))
-				break;
-			if (c1 != c2)
-				return 0;
-			match++;
-			name++;
-			namelen--;
-		}
-	} else {
-		for (;;) {
-			unsigned char c1 = *match;
-			unsigned char c2 = *name;
-			if (c1 == '\0' || is_glob_special(c1))
-				break;
-			if (c1 != c2)
-				return 0;
-			match++;
-			name++;
-			namelen--;
-		}
-	}
-
-
-	/*
-	 * If we don't match the matchstring exactly,
-	 * we need to match by fnmatch
-	 */
-	matchlen = strlen(match);
-	if (strncmp_icase(match, name, matchlen))
-		return !fnmatch_icase(match, name, 0) ? MATCHED_FNMATCH : 0;
-
-	if (namelen == matchlen)
-		return MATCHED_EXACTLY;
-	if (match[matchlen-1] == '/' || name[matchlen] == '/')
-		return MATCHED_RECURSIVELY;
-	return 0;
-}
-
-/*
- * Given a name and a list of pathspecs, see if the name matches
- * any of the pathspecs.  The caller is also interested in seeing
- * all pathspec matches some names it calls this function with
- * (otherwise the user could have mistyped the unmatched pathspec),
- * and a mark is left in seen[] array for pathspec element that
- * actually matched anything.
- */
-int match_pathspec(const char **pathspec, const char *name, int namelen,
-		int prefix, char *seen)
-{
-	int i, retval = 0;
-
-	if (!pathspec)
-		return 1;
-
-	name += prefix;
-	namelen -= prefix;
-
-	for (i = 0; pathspec[i] != NULL; i++) {
-		int how;
-		const char *match = pathspec[i] + prefix;
-		if (seen && seen[i] == MATCHED_EXACTLY)
-			continue;
-		how = match_one(match, name, namelen);
-		if (how) {
-			if (retval < how)
-				retval = how;
-			if (seen && seen[i] < how)
-				seen[i] = how;
-		}
-	}
-	return retval;
-}
-
-/*
- * Does 'match' match the given name?
- * A match is found if
- *
- * (1) the 'match' string is leading directory of 'name', or
- * (2) the 'match' string is a wildcard and matches 'name', or
- * (3) the 'match' string is exactly the same as 'name'.
- *
- * and the return value tells which case it was.
- *
- * It returns 0 when there is no match.
- */
 static int match_pathspec_item(const struct pathspec_item *item, int prefix,
 			       const char *name, int namelen)
 {
diff --git a/dir.h b/dir.h
index b51d2e9..44e24eb 100644
--- a/dir.h
+++ b/dir.h
@@ -68,7 +68,6 @@ struct dir_struct {
 #define MATCHED_EXACTLY 3
 extern int simple_length(const char *match);
 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);
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* Re: [PATCH v2 17/21] Convert refresh_index to take struct pathspec
From: Nguyen Thai Ngoc Duy @ 2013-01-11 14:19 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano
In-Reply-To: <1357903275-16804-18-git-send-email-pclouds@gmail.com>

On Fri, Jan 11, 2013 at 06:21:11PM +0700, Nguyễn Thái Ngọc Duy wrote:
> -	for (i = 0; i < specs; i++) {
> +	for (i = 0; i < pathspec->nr; i++) {
>  		if (!seen[i])
> -			die(_("pathspec '%s' did not match any files"), pathspec[i]);
> +			die(_("pathspec '%s' did not match any files"), pathspec->raw[i]);
>  	}

This needs the following fixup on top. I don't want to send another
reroll just a couple hours after I flooded git@vger. I did not plan to
work on the series this soon but somehow another problem got me back
here.

-- 8< --
diff --git a/builtin/add.c b/builtin/add.c
index 1235eb9..e1bcdb9 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -159,7 +159,8 @@ static void refresh(int verbose, const struct pathspec *pathspec)
 		      pathspec, seen, _("Unstaged changes after refreshing the index:"));
 	for (i = 0; i < pathspec->nr; i++) {
 		if (!seen[i])
-			die(_("pathspec '%s' did not match any files"), pathspec->raw[i]);
+			die(_("pathspec '%s' did not match any files"),
+			    pathspec->items[i].match);
 	}
         free(seen);
 }
-- 8< --

and the baaad reason: pathspec->items[] are sorted because of 86e4ca6
(tree_entry_interesting(): fix depth limit with overlapping pathspecs
- 2010-12-15). But raw[] are _not_. So raw[i] does not correspond to
item[i].

Now seen[] array returned from match_pathspec() has the order
corresponding to raw[]. On the other hand match_pathspec_depth()
returns seen[] corresponds to items[]. This patch converts
match_pathspec() to match_pathspec_depth() so we need to use the
correct pathspec array.

I'll put these explanation in the next reroll. And don't worry about
this subtle difference. My next email kills match_pathspec() for good.

^ permalink raw reply related

* Re: What's cooking in git.git (Jan 2013, #04; Wed, 9)
From: Duy Nguyen @ 2013-01-11 12:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmwwi3w5r.fsf@alter.siamese.dyndns.org>

On Thu, Jan 10, 2013 at 4:04 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Here are the topics that have been cooking.  Commits prefixed with
> '-' are only in 'pu' (proposed updates) while commits prefixed with
> '+' are in 'next'.

I see you start to use no-change commits as a way to keep notes in
'pu'. Isn't git notes designed for that? Just wanted to understand why
you chose that way and maybe we could improve git-notes a bit to fit
this purpose.
-- 
Duy

^ permalink raw reply

* Re: git checkout doesn't work?
From: Duy Nguyen @ 2013-01-11 12:52 UTC (permalink / raw)
  To: Ishayahu Lastov; +Cc: git, msysGit
In-Reply-To: <CAJ52sWkf+ZkLUdLw1ornVt5E2af3w1YSqhkOajTWxvTPKvkAJg@mail.gmail.com>

On Fri, Jan 11, 2013 at 1:46 PM, Ishayahu Lastov <meoc-it@mail.ru> wrote:
> This is my session on Win7 x64:
> Microsoft Windows [Version 6.1.7601]
> (c) Корпорация Майкрософт (Microsoft Corp.), 2009. Все права защищены.
>
> C:\Dropbox\Dropbox\Wesnoth\Apocryphs>cd Apokryphs.Orks
>
> C:\Dropbox\Dropbox\Wesnoth\Apocryphs\Apokryphs.Orks>git status
> # On branch master
> # Your branch is behind 'origin/master' by 3 commits, and can be fast-forwarded.
>
> #
> # Changes not staged for commit:
> #   (use "git add <file>..." to update what will be committed)
> #   (use "git checkout -- <file>..." to discard changes in working directory)
> #
> #       modified:   scenarios/01_NothernVillage.cfg
> #
> no changes added to commit (use "git add" and/or "git commit -a")
>
> C:\Dropbox\Dropbox\Wesnoth\Apocryphs\Apokryphs.Orks>cd scenarios
>
> C:\Dropbox\Dropbox\Wesnoth\Apocryphs\Apokryphs.Orks\scenarios>git status
> # On branch master
> # Your branch is behind 'origin/master' by 3 commits, and can be fast-forwarded.
>
> #
> # Changes not staged for commit:
> #   (use "git add <file>..." to update what will be committed)
> #   (use "git checkout -- <file>..." to discard changes in working directory)
> #
> #       modified:   01_NothernVillage.cfg
> #
> no changes added to commit (use "git add" and/or "git commit -a")
>
> C:\Dropbox\Dropbox\Wesnoth\Apocryphs\Apokryphs.Orks\scenarios>git checkout -- 01
> _NothernVillage.cfg
>
> C:\Dropbox\Dropbox\Wesnoth\Apocryphs\Apokryphs.Orks\scenarios>git status
> # On branch master
> # Your branch is behind 'origin/master' by 3 commits, and can be fast-forwarded.
>
> #
> # Changes not staged for commit:
> #   (use "git add <file>..." to update what will be committed)
> #   (use "git checkout -- <file>..." to discard changes in working directory)
> #
> #       modified:   01_NothernVillage.cfg
> #
> no changes added to commit (use "git add" and/or "git commit -a")
>
> C:\Dropbox\Dropbox\Wesnoth\Apocryphs\Apokryphs.Orks\scenarios>
>
> As I understand after last "git checkout" in "git status" I should see
> that I gave no changes. It looks like an bug, isn't it?

Yeah, perhaps it's file attributes. What does this say?

git diff --summary --stat
-- 
Duy

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

^ permalink raw reply

* [PATCH v2 21/21] Rename field "raw" to "_raw" in struct pathspec
From: Nguyễn Thái Ngọc Duy @ 2013-01-11 11:21 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1357903275-16804-1-git-send-email-pclouds@gmail.com>

This patch is essentially no-op. It helps catching new use of this
field though. This field is introduced as an intermediate step for the
pathspec conversion and will be removed eventually. At this stage no
more access sites should be introduced.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 archive.c          |  2 +-
 builtin/add.c      | 20 ++++++++++----------
 builtin/checkout.c |  2 +-
 builtin/commit.c   |  2 +-
 builtin/ls-files.c |  8 ++++----
 builtin/ls-tree.c  |  2 +-
 builtin/rm.c       |  2 +-
 cache.h            |  2 +-
 dir.c              | 22 +++++++++++-----------
 revision.c         |  4 ++--
 setup.c            |  6 +++---
 tree-diff.c        |  8 ++++----
 12 files changed, 40 insertions(+), 40 deletions(-)

diff --git a/archive.c b/archive.c
index 3caa31f..c52e525 100644
--- a/archive.c
+++ b/archive.c
@@ -229,7 +229,7 @@ static void parse_pathspec_arg(const char **pathspec,
 {
 	parse_pathspec(&ar_args->pathspec, PATHSPEC_FROMTOP, 0, "", pathspec);
 	if (ar_args->pathspec.nr) {
-		pathspec = ar_args->pathspec.raw;
+		pathspec = ar_args->pathspec._raw;
 		while (*pathspec) {
 			if (!path_exists(ar_args->tree, *pathspec))
 				die("path not found: %s", *pathspec);
diff --git a/builtin/add.c b/builtin/add.c
index 5d262eb..6b6a72e 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -165,7 +165,7 @@ static void refresh(int verbose, const struct pathspec *pathspec)
 		      pathspec, seen, _("Unstaged changes after refreshing the index:"));
 	for (i = 0; i < pathspec->nr; i++) {
 		if (!seen[i])
-			die(_("pathspec '%s' did not match any files"), pathspec->raw[i]);
+			die(_("pathspec '%s' did not match any files"), pathspec->_raw[i]);
 	}
         free(seen);
 }
@@ -387,7 +387,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 		return 0;
 	}
 	parse_pathspec(&pathspec, PATHSPEC_FROMTOP, 0, prefix, argv);
-	validate_pathspec(pathspec.raw, prefix);
+	validate_pathspec(pathspec._raw, prefix);
 
 	if (read_cache() < 0)
 		die(_("index file corrupt"));
@@ -406,7 +406,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 		/* This picks up the paths that are not tracked */
 		baselen = fill_directory(&dir, &pathspec);
 		if (pathspec.nr)
-			seen = prune_directory(&dir, pathspec.raw, baselen);
+			seen = prune_directory(&dir, pathspec._raw, baselen);
 	}
 
 	if (refresh_only) {
@@ -420,17 +420,17 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 
 		path_exclude_check_init(&check, &dir);
 		if (!seen)
-			seen = find_used_pathspec(pathspec.raw);
-		for (i = 0; pathspec.raw[i]; i++) {
-			if (!seen[i] && pathspec.raw[i][0]
-			    && !file_exists(pathspec.raw[i])) {
+			seen = find_used_pathspec(pathspec._raw);
+		for (i = 0; pathspec._raw[i]; i++) {
+			if (!seen[i] && pathspec._raw[i][0]
+			    && !file_exists(pathspec._raw[i])) {
 				if (ignore_missing) {
 					int dtype = DT_UNKNOWN;
-					if (path_excluded(&check, pathspec.raw[i], -1, &dtype))
-						dir_add_ignored(&dir, pathspec.raw[i], strlen(pathspec.raw[i]));
+					if (path_excluded(&check, pathspec._raw[i], -1, &dtype))
+						dir_add_ignored(&dir, pathspec._raw[i], strlen(pathspec._raw[i]));
 				} else
 					die(_("pathspec '%s' did not match any files"),
-					    pathspec.raw[i]);
+					    pathspec._raw[i]);
 			}
 		}
 		free(seen);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 648768e..716a949 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -253,7 +253,7 @@ static int checkout_paths(const struct checkout_opts *opts,
 
 	if (opts->patch_mode)
 		return run_add_interactive(revision, "--patch=checkout",
-					   opts->pathspec.raw);
+					   opts->pathspec._raw);
 
 	lock_file = xcalloc(1, sizeof(struct lock_file));
 
diff --git a/builtin/commit.c b/builtin/commit.c
index d79613d..876916c 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -192,7 +192,7 @@ static int list_paths(struct string_list *list, const char *with_tree,
 	m = xcalloc(1, pattern->nr);
 
 	if (with_tree) {
-		char *max_prefix = common_prefix(pattern->raw);
+		char *max_prefix = common_prefix(pattern->_raw);
 		overlay_tree_on_cache(with_tree, max_prefix ? max_prefix : prefix);
 		free(max_prefix);
 	}
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 79949de..e9caa42 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -367,11 +367,11 @@ int report_path_error(const char *ps_matched,
 		 * twice.  Do not barf on such a mistake.
 		 */
 		for (found_dup = other = 0;
-		     !found_dup && pathspec->raw[other];
+		     !found_dup && pathspec->_raw[other];
 		     other++) {
 			if (other == num || !ps_matched[other])
 				continue;
-			if (!strcmp(pathspec->raw[other], pathspec->raw[num]))
+			if (!strcmp(pathspec->_raw[other], pathspec->_raw[num]))
 				/*
 				 * Ok, we have a match already.
 				 */
@@ -380,7 +380,7 @@ int report_path_error(const char *ps_matched,
 		if (found_dup)
 			continue;
 
-		name = quote_path_relative(pathspec->raw[num], -1, &sb, prefix);
+		name = quote_path_relative(pathspec->_raw[num], -1, &sb, prefix);
 		error("pathspec '%s' did not match any file(s) known to git.",
 		      name);
 		errors++;
@@ -540,7 +540,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
 	strip_trailing_slash_from_submodules(&pathspec);
 
 	/* Find common prefix for all pathspec's */
-	max_prefix = common_prefix(pathspec.raw);
+	max_prefix = common_prefix(pathspec._raw);
 	max_prefix_len = max_prefix ? strlen(max_prefix) : 0;
 
 	/* Treat unmatching pathspec elements as errors */
diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index a78ba53..ebb587b 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -35,7 +35,7 @@ static int show_recursive(const char *base, int baselen, const char *pathname)
 	if (ls_options & LS_RECURSIVE)
 		return 1;
 
-	s = pathspec.raw;
+	s = pathspec._raw;
 	if (!s)
 		return 0;
 
diff --git a/builtin/rm.c b/builtin/rm.c
index b5edde8..d12311c 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -267,7 +267,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 	if (pathspec.nr) {
 		const char *match;
 		int seen_any = 0;
-		for (i = 0; (match = pathspec.raw[i]) != NULL ; i++) {
+		for (i = 0; (match = pathspec._raw[i]) != NULL ; i++) {
 			if (!seen[i]) {
 				if (!ignore_unmatch) {
 					die(_("pathspec '%s' did not match any files"),
diff --git a/cache.h b/cache.h
index 3e09a61..c5a408f 100644
--- a/cache.h
+++ b/cache.h
@@ -482,7 +482,7 @@ extern int ie_modified(const struct index_state *, struct cache_entry *, struct
 #define PATHSPEC_EMPTY_MATCH_ALL (1<<0) /* No args means match everything */
 
 struct pathspec {
-	const char **raw; /* get_pathspec() result, not freed by free_pathspec() */
+	const char **_raw; /* get_pathspec() result, not freed by free_pathspec() */
 	int nr;
 	unsigned int has_wildcard:1;
 	unsigned int recursive:1;
diff --git a/dir.c b/dir.c
index eb52913..aad180b 100644
--- a/dir.c
+++ b/dir.c
@@ -80,10 +80,10 @@ int fill_directory(struct dir_struct *dir, const struct pathspec *pathspec)
 	 * Calculate common prefix for the pathspec, and
 	 * use that to optimize the directory walk
 	 */
-	len = common_prefix_len(pathspec->raw);
+	len = common_prefix_len(pathspec->_raw);
 
 	/* Read the directory and prune it */
-	read_directory(dir, pathspec->nr ? pathspec->raw[0] : "", len, pathspec);
+	read_directory(dir, pathspec->nr ? pathspec->_raw[0] : "", len, pathspec);
 	return len;
 }
 
@@ -1218,7 +1218,7 @@ int read_directory(struct dir_struct *dir, const char *path, int len, const stru
 	if (has_symlink_leading_path(path, len))
 		return dir->nr;
 
-	simplify = create_simplify(pathspec ? pathspec->raw : NULL);
+	simplify = create_simplify(pathspec ? pathspec->_raw : NULL);
 	if (!len || treat_leading_path(dir, path, len, simplify))
 		read_directory_recursive(dir, path, len, 0, simplify);
 	free_simplify(simplify);
@@ -1417,7 +1417,7 @@ int init_pathspec(struct pathspec *pathspec, const char **paths)
 		return 0;
 	while (*p)
 		p++;
-	pathspec->raw = paths;
+	pathspec->_raw = paths;
 	pathspec->nr = p - paths;
 	if (!pathspec->nr)
 		return 0;
@@ -1444,7 +1444,7 @@ void strip_trailing_slash_from_submodules(struct pathspec *pathspec)
 {
 	int i;
 	for (i = 0; i < pathspec->nr; i++) {
-		const char *p = pathspec->raw[i];
+		const char *p = pathspec->_raw[i];
 		int len = strlen(p), pos;
 
 		if (len < 1 || p[len - 1] != '/')
@@ -1452,7 +1452,7 @@ void strip_trailing_slash_from_submodules(struct pathspec *pathspec)
 		pos = cache_name_pos(p, len - 1);
 		if (pos >= 0 && S_ISGITLINK(active_cache[pos]->ce_mode)) {
 			char *path = xstrndup(p, len - 1);
-			pathspec->raw[i] = path;
+			pathspec->_raw[i] = path;
 			pathspec->items[i].match = path;
 			pathspec->items[i].len = len - 1;
 			pathspec->items[i].nowildcard_len = simple_length(path);
@@ -1472,20 +1472,20 @@ void treat_gitlinks(struct pathspec *pathspec)
 			continue;
 
 		for (j = 0; j < pathspec->nr; j++) {
-			int len2 = strlen(pathspec->raw[j]);
-			if (len2 <= len || pathspec->raw[j][len] != '/' ||
-			    memcmp(ce->name, pathspec->raw[j], len))
+			int len2 = strlen(pathspec->_raw[j]);
+			if (len2 <= len || pathspec->_raw[j][len] != '/' ||
+			    memcmp(ce->name, pathspec->_raw[j], len))
 				continue;
 			if (len2 == len + 1) {
 				/* strip trailing slash */
 				char *path = xstrndup(ce->name, len);
-				pathspec->raw[j] = path;
+				pathspec->_raw[j] = path;
 				pathspec->items[j].match = path;
 				pathspec->items[j].len = len;
 				pathspec->items[j].nowildcard_len = simple_length(path);
 			} else
 				die (_("Path '%s' is in submodule '%.*s'"),
-				     pathspec->raw[j], len, ce->name);
+				     pathspec->_raw[j], len, ce->name);
 		}
 	}
 }
diff --git a/revision.c b/revision.c
index a044242..d89bb22 100644
--- a/revision.c
+++ b/revision.c
@@ -1885,12 +1885,12 @@ 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);
+		diff_tree_setup_paths(revs->prune_data._raw, &revs->pruning);
 		/* 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);
+			diff_tree_setup_paths(revs->prune_data._raw, &revs->diffopt);
 	}
 	if (revs->combine_merges)
 		revs->ignore_merges = 0;
diff --git a/setup.c b/setup.c
index a26b6c0..1182a0a 100644
--- a/setup.c
+++ b/setup.c
@@ -293,7 +293,7 @@ void parse_pathspec(struct pathspec *pathspec,
 		raw[0] = prefix;
 		raw[1] = NULL;
 		pathspec->nr = 1;
-		pathspec->raw = raw;
+		pathspec->_raw = raw;
 		return;
 	}
 
@@ -303,7 +303,7 @@ void parse_pathspec(struct pathspec *pathspec,
 
 	pathspec->nr = n;
 	pathspec->items = item = xmalloc(sizeof(*item) * n);
-	pathspec->raw = argv;
+	pathspec->_raw = argv;
 	prefixlen = prefix ? strlen(prefix) : 0;
 
 	for (i = 0; i < n; i++) {
@@ -327,7 +327,7 @@ const char **get_pathspec(const char *prefix, const char **pathspec)
 {
 	struct pathspec ps;
 	parse_pathspec(&ps, PATHSPEC_FROMTOP, 0, prefix, pathspec);
-	return ps.raw;
+	return ps._raw;
 }
 
 /*
diff --git a/tree-diff.c b/tree-diff.c
index ba01563..09bddd1 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -207,7 +207,7 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co
 	DIFF_OPT_SET(&diff_opts, RECURSIVE);
 	DIFF_OPT_SET(&diff_opts, FIND_COPIES_HARDER);
 	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
-	diff_opts.single_follow = opt->pathspec.raw[0];
+	diff_opts.single_follow = opt->pathspec._raw[0];
 	diff_opts.break_opt = opt->break_opt;
 	diff_opts.rename_score = opt->rename_score;
 	paths[0] = NULL;
@@ -228,15 +228,15 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co
 		 * the future!
 		 */
 		if ((p->status == 'R' || p->status == 'C') &&
-		    !strcmp(p->two->path, opt->pathspec.raw[0])) {
+		    !strcmp(p->two->path, opt->pathspec._raw[0])) {
 			/* Switch the file-pairs around */
 			q->queue[i] = choice;
 			choice = p;
 
 			/* Update the path we use from now on.. */
 			diff_tree_release_paths(opt);
-			opt->pathspec.raw[0] = xstrdup(p->one->path);
-			diff_tree_setup_paths(opt->pathspec.raw, opt);
+			opt->pathspec._raw[0] = xstrdup(p->one->path);
+			diff_tree_setup_paths(opt->pathspec._raw, opt);
 
 			/*
 			 * The caller expects us to return a set of vanilla
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH v2 20/21] Convert more init_pathspec() to parse_pathspec()
From: Nguyễn Thái Ngọc Duy @ 2013-01-11 11:21 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1357903275-16804-1-git-send-email-pclouds@gmail.com>

init_pathspec() was introduced to work with the result from
get_pathspec(). init_pathspec() will be removed eventually after
parse_pathspec() takes over, so that there is only place that
initializes struct pathspec.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 archive.c          |  2 +-
 builtin/log.c      |  2 +-
 builtin/ls-files.c | 10 ++++------
 diff-lib.c         |  2 +-
 merge-recursive.c  |  2 +-
 5 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/archive.c b/archive.c
index 530badb..3caa31f 100644
--- a/archive.c
+++ b/archive.c
@@ -218,7 +218,7 @@ static int path_exists(struct tree *tree, const char *path)
 	struct pathspec pathspec;
 	int ret;
 
-	init_pathspec(&pathspec, paths);
+	parse_pathspec(&pathspec, 0, 0, "", paths);
 	ret = read_tree_recursive(tree, "", 0, 0, &pathspec, reject_entry, NULL);
 	free_pathspec(&pathspec);
 	return ret != 0;
diff --git a/builtin/log.c b/builtin/log.c
index e7b7db1..495ae77 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -455,7 +455,7 @@ int cmd_show(int argc, const char **argv, const char *prefix)
 	init_grep_defaults();
 	git_config(git_log_config, NULL);
 
-	init_pathspec(&match_all, NULL);
+	memset(&match_all, 0, sizeof(match_all));
 	init_revisions(&rev, prefix);
 	rev.diff = 1;
 	rev.always_show_header = 1;
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 7bb637b..79949de 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -318,13 +318,11 @@ void overlay_tree_on_cache(const char *tree_name, const char *prefix)
 	}
 
 	if (prefix) {
-		static const char *(matchbuf[2]);
-		matchbuf[0] = prefix;
-		matchbuf[1] = NULL;
-		init_pathspec(&pathspec, matchbuf);
-		pathspec.items[0].nowildcard_len = pathspec.items[0].len;
+		static const char *(matchbuf[1]);
+		matchbuf[0] = NULL;
+		parse_pathspec(&pathspec, 0, 0, prefix, matchbuf);
 	} else
-		init_pathspec(&pathspec, NULL);
+		memset(&pathspec, 0, sizeof(pathspec));
 	if (read_tree(tree, 1, &pathspec))
 		die("unable to read tree entries %s", tree_name);
 
diff --git a/diff-lib.c b/diff-lib.c
index f35de0f..9c07f6a 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -500,7 +500,7 @@ int do_diff_cache(const unsigned char *tree_sha1, struct diff_options *opt)
 	struct rev_info revs;
 
 	init_revisions(&revs, NULL);
-	init_pathspec(&revs.prune_data, opt->pathspec.raw);
+	revs.prune_data = opt->pathspec;
 	revs.diffopt = *opt;
 
 	if (diff_cache(&revs, tree_sha1, NULL, 1))
diff --git a/merge-recursive.c b/merge-recursive.c
index d882060..cd95bdb 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -297,7 +297,7 @@ static int get_files_dirs(struct merge_options *o, struct tree *tree)
 {
 	int n;
 	struct pathspec match_all;
-	init_pathspec(&match_all, NULL);
+	memset(&match_all, 0, sizeof(match_all));
 	if (read_tree_recursive(tree, "", 0, 0, &match_all, save_files_dirs, o))
 		return 0;
 	n = o->current_file_set.nr + o->current_directory_set.nr;
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH v2 19/21] Convert add_files_to_cache to take struct pathspec
From: Nguyễn Thái Ngọc Duy @ 2013-01-11 11:21 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1357903275-16804-1-git-send-email-pclouds@gmail.com>


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/add.c    | 8 +++++---
 builtin/commit.c | 2 +-
 cache.h          | 2 +-
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index 0295cef..5d262eb 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -80,13 +80,15 @@ static void update_callback(struct diff_queue_struct *q,
 	}
 }
 
-int add_files_to_cache(const char *prefix, const char **pathspec, int flags)
+int add_files_to_cache(const char *prefix,
+		       const struct pathspec *pathspec, int flags)
 {
 	struct update_callback_data data;
 	struct rev_info rev;
 	init_revisions(&rev, prefix);
 	setup_revisions(0, NULL, &rev, NULL);
-	init_pathspec(&rev.prune_data, pathspec);
+	if (pathspec)
+		rev.prune_data = *pathspec;
 	rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
 	rev.diffopt.format_callback = update_callback;
 	data.flags = flags;
@@ -437,7 +439,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 
 	plug_bulk_checkin();
 
-	exit_status |= add_files_to_cache(prefix, pathspec.raw, flags);
+	exit_status |= add_files_to_cache(prefix, &pathspec, flags);
 
 	if (add_new_files)
 		exit_status |= add_files(&dir, flags);
diff --git a/builtin/commit.c b/builtin/commit.c
index 2fe6054..d79613d 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -329,7 +329,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
 	 */
 	if (all || (also && pathspec.nr)) {
 		fd = hold_locked_index(&index_lock, 1);
-		add_files_to_cache(also ? prefix : NULL, pathspec.raw, 0);
+		add_files_to_cache(also ? prefix : NULL, &pathspec, 0);
 		refresh_cache_or_die(refresh_flags);
 		update_main_cache_tree(WRITE_TREE_SILENT);
 		if (write_cache(fd, active_cache, active_nr) ||
diff --git a/cache.h b/cache.h
index 32298ba..3e09a61 100644
--- a/cache.h
+++ b/cache.h
@@ -1228,7 +1228,7 @@ void packet_trace_identity(const char *prog);
  * return 0 if success, 1 - if addition of a file failed and
  * ADD_FILES_IGNORE_ERRORS was specified in flags
  */
-int add_files_to_cache(const char *prefix, const char **pathspec, int flags);
+int add_files_to_cache(const char *prefix, const struct pathspec *pathspec, int flags);
 
 /* diff.c */
 extern int diff_auto_refresh_index;
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH v2 18/21] Convert {read,fill}_directory to take struct pathspec
From: Nguyễn Thái Ngọc Duy @ 2013-01-11 11:21 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1357903275-16804-1-git-send-email-pclouds@gmail.com>


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/add.c      |  2 +-
 builtin/clean.c    |  2 +-
 builtin/grep.c     |  2 +-
 builtin/ls-files.c |  2 +-
 dir.c              | 10 +++++-----
 dir.h              |  4 ++--
 wt-status.c        |  4 ++--
 7 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index 4e2b603..0295cef 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -402,7 +402,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 		}
 
 		/* This picks up the paths that are not tracked */
-		baselen = fill_directory(&dir, pathspec.raw);
+		baselen = fill_directory(&dir, &pathspec);
 		if (pathspec.nr)
 			seen = prune_directory(&dir, pathspec.raw, baselen);
 	}
diff --git a/builtin/clean.c b/builtin/clean.c
index 788ad8c..41c8cad 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -103,7 +103,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 
 	parse_pathspec(&pathspec, PATHSPEC_FROMTOP, 0, prefix, argv);
 
-	fill_directory(&dir, pathspec.raw);
+	fill_directory(&dir, &pathspec);
 
 	if (pathspec.nr)
 		seen = xmalloc(pathspec.nr);
diff --git a/builtin/grep.c b/builtin/grep.c
index 705f9ff..f370bad 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -522,7 +522,7 @@ static int grep_directory(struct grep_opt *opt, const struct pathspec *pathspec,
 	if (exc_std)
 		setup_standard_excludes(&dir);
 
-	fill_directory(&dir, pathspec->raw);
+	fill_directory(&dir, pathspec);
 	for (i = 0; i < dir.nr; i++) {
 		const char *name = dir.entries[i]->name;
 		int namelen = strlen(name);
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index be6e05d..7bb637b 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -216,7 +216,7 @@ static void show_files(struct dir_struct *dir)
 
 	/* For cached/deleted files we don't need to even do the readdir */
 	if (show_others || show_killed) {
-		fill_directory(dir, pathspec.raw);
+		fill_directory(dir, &pathspec);
 		if (show_others)
 			show_other_files(dir);
 		if (show_killed)
diff --git a/dir.c b/dir.c
index 4d1f71c..eb52913 100644
--- a/dir.c
+++ b/dir.c
@@ -72,7 +72,7 @@ char *common_prefix(const char **pathspec)
 	return len ? xmemdupz(*pathspec, len) : NULL;
 }
 
-int fill_directory(struct dir_struct *dir, const char **pathspec)
+int fill_directory(struct dir_struct *dir, const struct pathspec *pathspec)
 {
 	size_t len;
 
@@ -80,10 +80,10 @@ int fill_directory(struct dir_struct *dir, const char **pathspec)
 	 * Calculate common prefix for the pathspec, and
 	 * use that to optimize the directory walk
 	 */
-	len = common_prefix_len(pathspec);
+	len = common_prefix_len(pathspec->raw);
 
 	/* Read the directory and prune it */
-	read_directory(dir, pathspec ? *pathspec : "", len, pathspec);
+	read_directory(dir, pathspec->nr ? pathspec->raw[0] : "", len, pathspec);
 	return len;
 }
 
@@ -1211,14 +1211,14 @@ static int treat_leading_path(struct dir_struct *dir,
 	return rc;
 }
 
-int read_directory(struct dir_struct *dir, const char *path, int len, const char **pathspec)
+int read_directory(struct dir_struct *dir, const char *path, int len, const struct pathspec *pathspec)
 {
 	struct path_simplify *simplify;
 
 	if (has_symlink_leading_path(path, len))
 		return dir->nr;
 
-	simplify = create_simplify(pathspec);
+	simplify = create_simplify(pathspec ? pathspec->raw : NULL);
 	if (!len || treat_leading_path(dir, path, len, simplify))
 		read_directory_recursive(dir, path, len, 0, simplify);
 	free_simplify(simplify);
diff --git a/dir.h b/dir.h
index 1d4888b..b51d2e9 100644
--- a/dir.h
+++ b/dir.h
@@ -74,8 +74,8 @@ extern int match_pathspec_depth(const struct pathspec *pathspec,
 				int prefix, char *seen);
 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);
-extern int read_directory(struct dir_struct *, const char *path, int len, const char **pathspec);
+extern int fill_directory(struct dir_struct *dir, const struct pathspec *pathspec);
+extern int read_directory(struct dir_struct *, const char *path, int len, const struct pathspec *pathspec);
 
 extern int excluded_from_list(const char *pathname, int pathlen, const char *basename,
 			      int *dtype, struct exclude_list *el);
diff --git a/wt-status.c b/wt-status.c
index 13e6aba..2e1a62b 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -502,7 +502,7 @@ static void wt_status_collect_untracked(struct wt_status *s)
 			DIR_SHOW_OTHER_DIRECTORIES | DIR_HIDE_EMPTY_DIRECTORIES;
 	setup_standard_excludes(&dir);
 
-	fill_directory(&dir, s->pathspec.raw);
+	fill_directory(&dir, &s->pathspec);
 	for (i = 0; i < dir.nr; i++) {
 		struct dir_entry *ent = dir.entries[i];
 		if (cache_name_is_other(ent->name, ent->len) &&
@@ -514,7 +514,7 @@ static void wt_status_collect_untracked(struct wt_status *s)
 	if (s->show_ignored_files) {
 		dir.nr = 0;
 		dir.flags = DIR_SHOW_IGNORED | DIR_SHOW_OTHER_DIRECTORIES;
-		fill_directory(&dir, s->pathspec.raw);
+		fill_directory(&dir, &s->pathspec);
 		for (i = 0; i < dir.nr; i++) {
 			struct dir_entry *ent = dir.entries[i];
 			if (cache_name_is_other(ent->name, ent->len) &&
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH v2 17/21] Convert refresh_index to take struct pathspec
From: Nguyễn Thái Ngọc Duy @ 2013-01-11 11:21 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1357903275-16804-1-git-send-email-pclouds@gmail.com>


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/add.c    | 14 ++++++--------
 builtin/commit.c |  2 +-
 builtin/rm.c     |  2 +-
 cache.h          |  2 +-
 read-cache.c     |  5 +++--
 5 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index c8592fe..4e2b603 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -153,19 +153,17 @@ static char *prune_directory(struct dir_struct *dir, const char **pathspec, int
 	return seen;
 }
 
-static void refresh(int verbose, const char **pathspec)
+static void refresh(int verbose, const struct pathspec *pathspec)
 {
 	char *seen;
-	int i, specs;
+	int i;
 
-	for (specs = 0; pathspec[specs];  specs++)
-		/* nothing */;
-	seen = xcalloc(specs, 1);
+	seen = xcalloc(pathspec->nr, 1);
 	refresh_index(&the_index, verbose ? REFRESH_IN_PORCELAIN : REFRESH_QUIET,
 		      pathspec, seen, _("Unstaged changes after refreshing the index:"));
-	for (i = 0; i < specs; i++) {
+	for (i = 0; i < pathspec->nr; i++) {
 		if (!seen[i])
-			die(_("pathspec '%s' did not match any files"), pathspec[i]);
+			die(_("pathspec '%s' did not match any files"), pathspec->raw[i]);
 	}
         free(seen);
 }
@@ -410,7 +408,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 	}
 
 	if (refresh_only) {
-		refresh(verbose, pathspec.raw);
+		refresh(verbose, &pathspec);
 		goto finish;
 	}
 
diff --git a/builtin/commit.c b/builtin/commit.c
index 8777c19..2fe6054 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1208,7 +1208,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 		       prefix, argv);
 
 	read_cache_preload(&s.pathspec);
-	refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, s.pathspec.raw, NULL, NULL);
+	refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, &s.pathspec, NULL, NULL);
 
 	fd = hold_locked_index(&index_lock, 0);
 	if (0 <= fd)
diff --git a/builtin/rm.c b/builtin/rm.c
index d719d95..b5edde8 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -250,7 +250,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 	}
 
 	parse_pathspec(&pathspec, PATHSPEC_FROMTOP, 0, prefix, argv);
-	refresh_index(&the_index, REFRESH_QUIET, pathspec.raw, NULL, NULL);
+	refresh_index(&the_index, REFRESH_QUIET, &pathspec, NULL, NULL);
 
 	seen = NULL;
 	seen = xcalloc(pathspec.nr, 1);
diff --git a/cache.h b/cache.h
index 40eaa04..32298ba 100644
--- a/cache.h
+++ b/cache.h
@@ -515,7 +515,7 @@ extern void fill_stat_cache_info(struct cache_entry *ce, struct stat *st);
 #define REFRESH_IGNORE_MISSING	0x0008	/* ignore non-existent */
 #define REFRESH_IGNORE_SUBMODULES	0x0010	/* ignore submodules */
 #define REFRESH_IN_PORCELAIN	0x0020	/* user friendly output, not "needs update" */
-extern int refresh_index(struct index_state *, unsigned int flags, const char **pathspec, char *seen, const char *header_msg);
+extern int refresh_index(struct index_state *, unsigned int flags, const struct pathspec *pathspec, char *seen, const char *header_msg);
 
 struct lock_file {
 	struct lock_file *next;
diff --git a/read-cache.c b/read-cache.c
index fda78bc..dec2ba6 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1093,7 +1093,8 @@ static void show_file(const char * fmt, const char * name, int in_porcelain,
 	printf(fmt, name);
 }
 
-int refresh_index(struct index_state *istate, unsigned int flags, const char **pathspec,
+int refresh_index(struct index_state *istate, unsigned int flags,
+		  const struct pathspec *pathspec,
 		  char *seen, const char *header_msg)
 {
 	int i;
@@ -1128,7 +1129,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p
 			continue;
 
 		if (pathspec &&
-		    !match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, seen))
+		    !match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, seen))
 			filtered = 1;
 
 		if (ce_stage(ce)) {
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH v2 16/21] Convert report_path_error to take struct pathspec
From: Nguyễn Thái Ngọc Duy @ 2013-01-11 11:21 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1357903275-16804-1-git-send-email-pclouds@gmail.com>


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/checkout.c |  2 +-
 builtin/commit.c   | 14 ++++++--------
 builtin/ls-files.c | 14 ++++++++------
 cache.h            |  2 +-
 4 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index a7ddb35..648768e 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -273,7 +273,7 @@ static int checkout_paths(const struct checkout_opts *opts,
 		match_pathspec_depth(&opts->pathspec, ce->name, ce_namelen(ce), 0, ps_matched);
 	}
 
-	if (report_path_error(ps_matched, opts->pathspec.raw, opts->prefix))
+	if (report_path_error(ps_matched, &opts->pathspec, opts->prefix))
 		return 1;
 
 	/* "checkout -m path" to recreate conflicted state */
diff --git a/builtin/commit.c b/builtin/commit.c
index 069d853..8777c19 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -181,20 +181,18 @@ static int commit_index_files(void)
  * and return the paths that match the given pattern in list.
  */
 static int list_paths(struct string_list *list, const char *with_tree,
-		      const char *prefix, const char **pattern)
+		      const char *prefix, const struct pathspec *pattern)
 {
 	int i;
 	char *m;
 
-	if (!pattern)
+	if (!pattern->nr)
 		return 0;
 
-	for (i = 0; pattern[i]; i++)
-		;
-	m = xcalloc(1, i);
+	m = xcalloc(1, pattern->nr);
 
 	if (with_tree) {
-		char *max_prefix = common_prefix(pattern);
+		char *max_prefix = common_prefix(pattern->raw);
 		overlay_tree_on_cache(with_tree, max_prefix ? max_prefix : prefix);
 		free(max_prefix);
 	}
@@ -205,7 +203,7 @@ static int list_paths(struct string_list *list, const char *with_tree,
 
 		if (ce->ce_flags & CE_UPDATE)
 			continue;
-		if (!match_pathspec(pattern, ce->name, ce_namelen(ce), 0, m))
+		if (!match_pathspec_depth(pattern, ce->name, ce_namelen(ce), 0, m))
 			continue;
 		item = string_list_insert(list, ce->name);
 		if (ce_skip_worktree(ce))
@@ -395,7 +393,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
 
 	memset(&partial, 0, sizeof(partial));
 	partial.strdup_strings = 1;
-	if (list_paths(&partial, !current_head ? NULL : "HEAD", prefix, pathspec.raw))
+	if (list_paths(&partial, !current_head ? NULL : "HEAD", prefix, &pathspec))
 		exit(1);
 
 	discard_cache();
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 9336abd..be6e05d 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -349,7 +349,9 @@ void overlay_tree_on_cache(const char *tree_name, const char *prefix)
 	}
 }
 
-int report_path_error(const char *ps_matched, const char **pathspec, const char *prefix)
+int report_path_error(const char *ps_matched,
+		      const struct pathspec *pathspec,
+		      const char *prefix)
 {
 	/*
 	 * Make sure all pathspec matched; otherwise it is an error.
@@ -357,7 +359,7 @@ int report_path_error(const char *ps_matched, const char **pathspec, const char
 	struct strbuf sb = STRBUF_INIT;
 	const char *name;
 	int num, errors = 0;
-	for (num = 0; pathspec[num]; num++) {
+	for (num = 0; num < pathspec->nr; num++) {
 		int other, found_dup;
 
 		if (ps_matched[num])
@@ -367,11 +369,11 @@ int report_path_error(const char *ps_matched, const char **pathspec, const char
 		 * twice.  Do not barf on such a mistake.
 		 */
 		for (found_dup = other = 0;
-		     !found_dup && pathspec[other];
+		     !found_dup && pathspec->raw[other];
 		     other++) {
 			if (other == num || !ps_matched[other])
 				continue;
-			if (!strcmp(pathspec[other], pathspec[num]))
+			if (!strcmp(pathspec->raw[other], pathspec->raw[num]))
 				/*
 				 * Ok, we have a match already.
 				 */
@@ -380,7 +382,7 @@ int report_path_error(const char *ps_matched, const char **pathspec, const char
 		if (found_dup)
 			continue;
 
-		name = quote_path_relative(pathspec[num], -1, &sb, prefix);
+		name = quote_path_relative(pathspec->raw[num], -1, &sb, prefix);
 		error("pathspec '%s' did not match any file(s) known to git.",
 		      name);
 		errors++;
@@ -572,7 +574,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
 
 	if (ps_matched) {
 		int bad;
-		bad = report_path_error(ps_matched, pathspec.raw, prefix);
+		bad = report_path_error(ps_matched, &pathspec, prefix);
 		if (bad)
 			fprintf(stderr, "Did you forget to 'git add'?\n");
 
diff --git a/cache.h b/cache.h
index c594ded..40eaa04 100644
--- a/cache.h
+++ b/cache.h
@@ -1262,7 +1262,7 @@ extern int ws_blank_line(const char *line, int len, unsigned ws_rule);
 #define ws_tab_width(rule)     ((rule) & WS_TAB_WIDTH_MASK)
 
 /* ls-files */
-int report_path_error(const char *ps_matched, const char **pathspec, const char *prefix);
+int report_path_error(const char *ps_matched, const struct pathspec *pathspec, const char *prefix);
 void overlay_tree_on_cache(const char *tree_name, const char *prefix);
 
 char *alias_lookup(const char *alias);
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH v2 15/21] checkout: convert read_tree_some to take struct pathspec
From: Nguyễn Thái Ngọc Duy @ 2013-01-11 11:21 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1357903275-16804-1-git-send-email-pclouds@gmail.com>


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

diff --git a/builtin/checkout.c b/builtin/checkout.c
index aa399d6..a7ddb35 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -82,12 +82,9 @@ static int update_some(const unsigned char *sha1, const char *base, int baselen,
 	return 0;
 }
 
-static int read_tree_some(struct tree *tree, const char **pathspec)
+static int read_tree_some(struct tree *tree, const struct pathspec *pathspec)
 {
-	struct pathspec ps;
-	init_pathspec(&ps, pathspec);
-	read_tree_recursive(tree, "", 0, 0, &ps, update_some, NULL);
-	free_pathspec(&ps);
+	read_tree_recursive(tree, "", 0, 0, pathspec, update_some, NULL);
 
 	/* update the index with the given tree's info
 	 * for all args, expanding wildcards, and exit
@@ -265,7 +262,7 @@ static int checkout_paths(const struct checkout_opts *opts,
 		return error(_("corrupt index file"));
 
 	if (opts->source_tree)
-		read_tree_some(opts->source_tree, opts->pathspec.raw);
+		read_tree_some(opts->source_tree, &opts->pathspec);
 
 	ps_matched = xcalloc(1, opts->pathspec.nr);
 
diff --git a/tree.c b/tree.c
index 62fed63..ff72f67 100644
--- a/tree.c
+++ b/tree.c
@@ -47,7 +47,7 @@ static int read_one_entry_quick(const unsigned char *sha1, const char *base, int
 }
 
 static int read_tree_1(struct tree *tree, struct strbuf *base,
-		       int stage, struct pathspec *pathspec,
+		       int stage, const struct pathspec *pathspec,
 		       read_tree_fn_t fn, void *context)
 {
 	struct tree_desc desc;
@@ -116,7 +116,7 @@ static int read_tree_1(struct tree *tree, struct strbuf *base,
 
 int read_tree_recursive(struct tree *tree,
 			const char *base, int baselen,
-			int stage, struct pathspec *pathspec,
+			int stage, const struct pathspec *pathspec,
 			read_tree_fn_t fn, void *context)
 {
 	struct strbuf sb = STRBUF_INIT;
diff --git a/tree.h b/tree.h
index 69bcb5e..9dc90ba 100644
--- a/tree.h
+++ b/tree.h
@@ -25,7 +25,7 @@ typedef int (*read_tree_fn_t)(const unsigned char *, const char *, int, const ch
 
 extern int read_tree_recursive(struct tree *tree,
 			       const char *base, int baselen,
-			       int stage, struct pathspec *pathspec,
+			       int stage, const struct pathspec *pathspec,
 			       read_tree_fn_t fn, void *context);
 
 extern int read_tree(struct tree *tree, int stage, struct pathspec *pathspec);
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH v2 14/21] Convert unmerge_cache to take struct pathspec
From: Nguyễn Thái Ngọc Duy @ 2013-01-11 11:21 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1357903275-16804-1-git-send-email-pclouds@gmail.com>


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/checkout.c | 2 +-
 rerere.c           | 2 +-
 resolve-undo.c     | 4 ++--
 resolve-undo.h     | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 00910dc..aa399d6 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -281,7 +281,7 @@ static int checkout_paths(const struct checkout_opts *opts,
 
 	/* "checkout -m path" to recreate conflicted state */
 	if (opts->merge)
-		unmerge_cache(opts->pathspec.raw);
+		unmerge_cache(&opts->pathspec);
 
 	/* Any unmerged paths? */
 	for (pos = 0; pos < active_nr; pos++) {
diff --git a/rerere.c b/rerere.c
index f8ddf85..9d149fa 100644
--- a/rerere.c
+++ b/rerere.c
@@ -666,7 +666,7 @@ int rerere_forget(struct pathspec *pathspec)
 
 	fd = setup_rerere(&merge_rr, RERERE_NOAUTOUPDATE);
 
-	unmerge_cache(pathspec->raw);
+	unmerge_cache(pathspec);
 	find_conflict(&conflict);
 	for (i = 0; i < conflict.nr; i++) {
 		struct string_list_item *it = &conflict.items[i];
diff --git a/resolve-undo.c b/resolve-undo.c
index 72b4612..1bfece2 100644
--- a/resolve-undo.c
+++ b/resolve-undo.c
@@ -156,7 +156,7 @@ int unmerge_index_entry_at(struct index_state *istate, int pos)
 	return unmerge_index_entry_at(istate, pos);
 }
 
-void unmerge_index(struct index_state *istate, const char **pathspec)
+void unmerge_index(struct index_state *istate, const struct pathspec *pathspec)
 {
 	int i;
 
@@ -165,7 +165,7 @@ void unmerge_index(struct index_state *istate, const char **pathspec)
 
 	for (i = 0; i < istate->cache_nr; i++) {
 		struct cache_entry *ce = istate->cache[i];
-		if (!match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, NULL))
+		if (!match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, NULL))
 			continue;
 		i = unmerge_index_entry_at(istate, i);
 	}
diff --git a/resolve-undo.h b/resolve-undo.h
index 8458769..81e8803 100644
--- a/resolve-undo.h
+++ b/resolve-undo.h
@@ -11,6 +11,6 @@ extern void resolve_undo_write(struct strbuf *, struct string_list *);
 extern struct string_list *resolve_undo_read(const char *, unsigned long);
 extern void resolve_undo_clear_index(struct index_state *);
 extern int unmerge_index_entry_at(struct index_state *, int);
-extern void unmerge_index(struct index_state *, const char **);
+extern void unmerge_index(struct index_state *, const struct pathspec *);
 
 #endif
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related


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