* Re: git send-email should not allow 'y' for in-reply-to
From: Antoine Pelisse @ 2013-01-11 21:53 UTC (permalink / raw)
To: Jeff King; +Cc: Matt Seitz (matseitz), git@vger.kernel.org
In-Reply-To: <20130111212325.GA18193@sigill.intra.peff.net>
On Fri, Jan 11, 2013 at 10:23 PM, Jeff King <peff@peff.net> wrote:
> On Fri, Jan 11, 2013 at 08:13:57PM +0000, Matt Seitz (matseitz) wrote:
>
>> > > How about "What Message-ID to use as In-Reply-To for the first email?"
>> > > or "Provide the Message-ID to use as In-Reply-To for the first
>> > > email:".
>> >
>> > seem fine to me. Maybe somebody who has been confused by it can offer
>> > more. At any rate, patches welcome.
>>
>> Suggestion: "Message-ID to use as In-Reply-To for the first email:".
>>
>> Simple and unlikely to generate a "y" or "n" response. Putting
>> "Message-ID" first makes it more obvious what data is being asked for
>> by this prompt.
>
> You'd think. But the existing message that has been causing problems is:
>
> Message-ID to be used as In-Reply-To for the first email?
>
> which is more or less what you are proposing. I do think a colon rather
> than a question mark helps indicate that the response is not yes/no.
That is true.
I'm definitely not a wording person, but assuming people who make the
mistake probably don't read the whole sentence out of laziness (that
might be somehow extreme though ;), starting it with "what" makes it
obvious at first sight that you can't answer yes/no.
That is not true if the message starts with Message-ID .. which
doesn't look like a question. Now it feels like you have agree or not.
Antoine,
^ permalink raw reply
* Re: git send-email should not allow 'y' for in-reply-to
From: Jeff King @ 2013-01-11 21:23 UTC (permalink / raw)
To: Matt Seitz (matseitz); +Cc: git@vger.kernel.org
In-Reply-To: <A0DB01D693D8EF439496BC8B037A0AEF322039A7@xmb-rcd-x15.cisco.com>
On Fri, Jan 11, 2013 at 08:13:57PM +0000, Matt Seitz (matseitz) wrote:
> > > How about "What Message-ID to use as In-Reply-To for the first email?"
> > > or "Provide the Message-ID to use as In-Reply-To for the first
> > > email:".
> >
> > seem fine to me. Maybe somebody who has been confused by it can offer
> > more. At any rate, patches welcome.
>
> Suggestion: "Message-ID to use as In-Reply-To for the first email:".
>
> Simple and unlikely to generate a "y" or "n" response. Putting
> "Message-ID" first makes it more obvious what data is being asked for
> by this prompt.
You'd think. But the existing message that has been causing problems is:
Message-ID to be used as In-Reply-To for the first email?
which is more or less what you are proposing. I do think a colon rather
than a question mark helps indicate that the response is not yes/no.
-Peff
^ permalink raw reply
* Re: [PATCH] git-completion.bash: Silence not a valid object errors
From: Manlio Perillo @ 2013-01-11 21:12 UTC (permalink / raw)
To: Dylan Smith; +Cc: git
In-Reply-To: <alpine.DEB.2.02.1301110304220.26739@antec>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Il 11/01/2013 09:06, Dylan Smith ha scritto:
> 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.
>
I reported the problem a few weeks ago; thanks.
> [...]
Regards Manlio
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAlDwgCAACgkQscQJ24LbaUTAxgCfZbB8X8IaTZAcT8iTs1XIILBJ
72MAn2zlh3xbRa/wjq1WyA2yOiAlaCr7
=dMN7
-----END PGP SIGNATURE-----
^ permalink raw reply
* Re: git-archive fails against smart-http repos
From: Jeff King @ 2013-01-11 20:58 UTC (permalink / raw)
To: Bruce Lysik; +Cc: git
In-Reply-To: <6014ED12-17F9-4D57-927F-6AFCD8A51C9D@apple.com>
On Wed, Jan 09, 2013 at 10:52:48AM -0800, Bruce Lysik wrote:
> Trying to run git-archive fails against smart-http based repos. Example:
>
> $ git archive --verbose --format=zip --remote=http://code.toofishes.net/git/dan/initscripts.git
> fatal: Operation not supported by protocol.
> Unexpected end of command stream
>
> This problem was brought up against my internal repos as well.
Right. Neither the client nor server for the http transport knows how to
handle the "git-upload-archive" service (as opposed to the regular
"git-upload-pack" or "git-receive-pack" services). I don't think there's
anything technical standing in the way; it is has simply never been
implemented.
Currently, you can do remote git-archive only locally, via ssh, or over
git:// (but then only if the server side has explicitly enabled it).
Patches welcome.
-Peff
^ permalink raw reply
* Re: git send-email should not allow 'y' for in-reply-to
From: Matt Seitz (matseitz) @ 2013-01-11 20:13 UTC (permalink / raw)
To: git@vger.kernel.org
"Jeff King" <peff@peff.net> wrote in message news:<20130111185417.GA12852@sigill.intra.peff.net>...
> On Fri, Jan 11, 2013 at 10:43:39AM -0800, Hilco Wijbenga wrote:
>
>
> > How about "What Message-ID to use as In-Reply-To for the first email?"
> > or "Provide the Message-ID to use as In-Reply-To for the first
> > email:".
>
> seem fine to me. Maybe somebody who has been confused by it can offer
> more. At any rate, patches welcome.
Suggestion: "Message-ID to use as In-Reply-To for the first email:".
Simple and unlikely to generate a "y" or "n" response. Putting "Message-ID" first makes it more obvious what data is being asked for by this prompt.
^ permalink raw reply
* [BUG]:Git doesn't work with Sock5 proxy on MAC
From: Herry Wang @ 2013-01-11 20:22 UTC (permalink / raw)
To: git
On Thu, Jan 10, 2013 at 9:51 PM, Herry Wang <tech.herry@gmail.com> wrote:
>
> OS:
>
> Darwin ... 11.4.2 Darwin Kernel Version 11.4.2: Thu Aug 23 16:25:48 PDT
> 2012; root:xnu-1699.32.7~1/RELEASE_X86_64 x86_64
>
> Git: version: (via homebrew)
>
> git --version
> git version 1.8.0
>
> curl version:
>
> curl --version
> curl 7.28.1 (x86_64-apple-darwin11.4.2)
>
>
> git clone http://herry@stash.somesite/a.git
> Cloning into 'a'...
> error: Empty reply from server while accessing
> ....git/info/refs?service=git-upload-pack
> fatal: HTTP request failed
>
> i tried export http_proxy=socks5://ip:port, all_proxy=socks5://, neither
> of them works.
> I also configure socks proxy in ~/.curlrc, git is not working well.
> However, curl is doing well with curlrc config.
> From the trace, looks like git is just put the http request via proxy
> host. But according with socks protocol, it should have some headers.
>
>
> Interesting thing is, http_proxy way is working perfectly on my Ubuntu
> enviroment.
>
>
> Thanks
> Herry
>
^ permalink raw reply
* Re: Version 1.8.1 does not compile on Cygwin 1.7.14
From: Alex Riesen @ 2013-01-11 20:17 UTC (permalink / raw)
To: Mark Levedahl
Cc: Junio C Hamano, Jason Pyeron, git, Jonathan Nieder,
Torsten Bögershausen, Stephen & Linda Smith, Eric Blake
In-Reply-To: <CALxABCYHRp17rcoOca1xWG9S19fq2rotz8FEKo09jNdrgMLiyQ@mail.gmail.com>
On Fri, Jan 11, 2013 at 9:08 PM, Alex Riesen <raa.lkml@gmail.com> wrote:
> This short discussion on GitHub (file git-compat-util.h) might be relevant:
>
> https://github.com/msysgit/git/commit/435bdf8c7ffa493f8f6f2e8f329f8cc22db16ce6#commitcomment-2407194
>
> The change suggested there (to remove an inclusion of windows.h in
> git-compat-util.h) might simplify the solution a little. Might even
> remove the need for auto-configuration in Makefile (worked for me).
Just to be clear, the change is this:
diff --git a/git-compat-util.h b/git-compat-util.h
index 4a1979f..780a919 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -85,12 +85,6 @@
#define _NETBSD_SOURCE 1
#define _SGI_SOURCE 1
-#ifdef WIN32 /* Both MinGW and MSVC */
-#define WIN32_LEAN_AND_MEAN /* stops windows.h including winsock.h */
-#include <winsock2.h>
-#include <windows.h>
-#endif
-
#include <unistd.h>
#include <stdio.h>
#include <sys/stat.h>
^ permalink raw reply related
* Re: Version 1.8.1 does not compile on Cygwin 1.7.14
From: Alex Riesen @ 2013-01-11 20:08 UTC (permalink / raw)
To: Mark Levedahl
Cc: Junio C Hamano, Jason Pyeron, git, Jonathan Nieder,
Torsten Bögershausen, Stephen & Linda Smith, Eric Blake
In-Reply-To: <50EB8EB5.6080204@gmail.com>
This short discussion on GitHub (file git-compat-util.h) might be relevant:
https://github.com/msysgit/git/commit/435bdf8c7ffa493f8f6f2e8f329f8cc22db16ce6#commitcomment-2407194
The change suggested there (to remove an inclusion of windows.h in
git-compat-util.h) might simplify the solution a little. Might even
remove the need for auto-configuration in Makefile (worked for me).
^ permalink raw reply
* Re: [PATCH] cvsimport: rewrite to use cvsps 3.x to fix major bugs
From: Junio C Hamano @ 2013-01-11 19:27 UTC (permalink / raw)
To: esr; +Cc: git
In-Reply-To: <20130111185818.GA30398@thyrsus.com>
"Eric S. Raymond" <esr@thyrsus.com> writes:
> Junio C Hamano <gitster@pobox.com>:
> ...
> The other is a design-level problem - these options were a bad idea to
> begin with. In earlier list mail I said
>
> An example of the batchiness mistake close to home is the -m and -M
> options in the old version of cvsimport. It takes human judgment
> looking at the whole commit DAG in gitspace to decide what merge
> points would best express the (as you say, sometimes ambiguous) CVS
> history - what's needed is a scalpel and sutures in a surgeon's hand,
> not a regexp hammer.
>
> One specific problem with the regexp hammer is false-positive matches
> leading to unintended merges.
Yeah, it is OK to _discourage_ its use, but to me it looks like that
the above is a fairly subjective policy decision, not something I
should let you impose on the users of the old cvsimport, which you
do not seem to even treat as your users.
>> 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?
>
> Only to the extent that -o/-m/-M are really important, which I doubt.
> But that might be fixable, and I'll put it on the to-do list.
>
>> 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.
>
> If you think that, you have failed to understand just how broken and
> dangerous the old combination is. None of the details you've called
> out are "major" by any stretch of the imagination compared to it
> silently botching the translation of repositories.
The "major" in my sentence was from your description (the bugs you
fixed), and not about the new ones you still have in this draft. I
did not mean to say that you are trading fixes to "major" bugs with
different "major" bugs.
Insecure quoting of parameters is much easier to fix; it does need
to be addressed, though.
It is just that looking at the state of the patch as submitted left
me with a feeling that this topic needs a lot more time to mature
than I previously was led to believe by your earlier messages, which
made me someaht sad.
^ permalink raw reply
* Re: [PATCH] cvsimport: rewrite to use cvsps 3.x to fix major bugs
From: Junio C Hamano @ 2013-01-11 19:17 UTC (permalink / raw)
To: esr; +Cc: git
In-Reply-To: <20130111185818.GA30398@thyrsus.com>
"Eric S. Raymond" <esr@thyrsus.com> writes:
> Junio C Hamano <gitster@pobox.com>:
>> I think the prevalent style in this script is to write "print"
>> without parentheses:
>>
>> print STDERR "msg\n";
>
> That can be easily fixed.
>
>> This looks lazy and unsafe quoting. Is there anything that makes
>> sure repository path does not contain a single quote?
>
> No. But...wait, checking...the Perl code didn't have the analogous
> check, so there's no increased vulnerability here. I'll put it on the
> to-do list for after I ship parsecvs.
I checked before I sent that review, and as far as I could tell, it
was fairly consistently avoiding the lazy and insecure forms, e.g.
system("com mand " . $param);
open($fh, "com mand " . $param . " |"); while (<$fh>) { ... }
but used the more sequre list form, e.g.
system(qw(com mand), $param);
open($fh, "-|", qw(com mand), $param); while (<$fh>) { ... }
But of course there may be some places that were careless that I
didn't spot (and previous reviewers of the current cvsimport
didn't).
^ permalink raw reply
* Re: parsecvs has been salvaged
From: Eric S. Raymond @ 2013-01-11 19:02 UTC (permalink / raw)
To: Bart Massey; +Cc: git, Keith Packard
In-Reply-To: <CAA6gtpnDrVOfiX-bQFS2X91wsS705b60YST8DwuDaibjkYU9vg@mail.gmail.com>
Bart Massey <bart@cs.pdx.edu>:
> 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
And now it has that -R option and correctly interprets the timezone field.
(I've been busy this morning.) I'm working on the no-commitids warning now.
Oh, and it now has...actual documentation, too. :-)
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
^ permalink raw reply
* Re: [PATCH] cvsimport: rewrite to use cvsps 3.x to fix major bugs
From: Eric S. Raymond @ 2013-01-11 18:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7va9sfd6lk.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com>:
> I think the prevalent style in this script is to write "print"
> without parentheses:
>
> print STDERR "msg\n";
That can be easily fixed.
> This looks lazy and unsafe quoting. Is there anything that makes
> sure repository path does not contain a single quote?
No. But...wait, checking...the Perl code didn't have the analogous
check, so there's no increased vulnerability here. I'll put it on the
to-do list for after I ship parsecvs.
> > + 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?
Yes. The correct fix is to simplify cvs2git's rather baroque command-line
interface. Michael Haggerty has accepted that patch. Soon that line will
look like this:
return "cvs2git --quiet --quiet {0} {1}".format(self.opts, self.modulepath)
Older versions of cvs2git will terminate cleanly with an error message
when called this way.
> > + elif opt == '-o':
> > + sys.stderr.write("git cvsimport: -o is no longer supported.\n")
> > + sys.exit(1)
>
> Isn't this a regression?
It would be if the -o behavior were consistent and decently
documented. When I tested this option with the Perl version I got no
result. Possibly my usage was incorrect; if anyone can be found who
actually understands how it's supposed to work in detail and will tell
me, I can probably support it.
> > + 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?
Sorry, I don't know where you got that idea. I don't think the general merge
detection that would need is possible even in principle.
> Otherwise it looks like a regression to me.
There are two reasons -m and -M aren't supported.
One is implementation-level. The wrapper script no longer deals with
individual files or changesets or branches; it relies on the
conversion engine to do all that. (As it should - the old design was
a mess with lots of stuff being done at the wrong level.) But the
conversion engines don't implement -m or -M, and aren't ever going to
(see next paragraph).
The other is a design-level problem - these options were a bad idea to
begin with. In earlier list mail I said
An example of the batchiness mistake close to home is the -m and -M
options in the old version of cvsimport. It takes human judgment
looking at the whole commit DAG in gitspace to decide what merge
points would best express the (as you say, sometimes ambiguous) CVS
history - what's needed is a scalpel and sutures in a surgeon's hand,
not a regexp hammer.
One specific problem with the regexp hammer is false-positive matches
leading to unintended merges.
That's why I won't implement these in cvsps or parsecvs. Instead I've
just added DAG visualization via graphviz in reposurgeon, so a human
can quickly see candidate merges in the visualization and do them by
hand. This is better and safer.
> 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?
Only to the extent that -o/-m/-M are really important, which I doubt.
But that might be fixable, and I'll put it on the to-do list.
> 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.
If you think that, you have failed to understand just how broken and
dangerous the old combination is. None of the details you've called
out are "major" by any stretch of the imagination compared to it
silently botching the translation of repositories.
Also bear in mind that leaving the old Perl code in place is not going
to be a viable option for more than a few months out. As cvsps-3.x
propagates to the distros what you have is going to stop even its
current pretense of working.
Finally...my own purposes are fulfilled by having CVS exporters that can
do a decent job of front-ending for reposurgeon. Rewriting git's
wrapper script was extra work that I took on only because I wanted to
be friendly to the git project, *but*...
...there is a limit to the amount of what I consider pointless
hoop-jumping that friendliness will buy you, and the 2.x fallback eas
already pushing that limit. Tread a little more gently, Junio; I've
put in a lot of hard, boring work on git-cvsimport over the last two
weeks when I would rather have been doing other things, and my
patience for being nit-picked without appreciation or reward has a
correspondingly low limit. We'll both be happier if you don't reach
it.
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
^ permalink raw reply
* Re: git send-email should not allow 'y' for in-reply-to
From: Jeff King @ 2013-01-11 18:54 UTC (permalink / raw)
To: Hilco Wijbenga; +Cc: Eric Blake, git, libvir-list
In-Reply-To: <CAE1pOi0zc+d6d9Y4KViX24qHgc1WL9atmRuyygorX_DQMj69Hg@mail.gmail.com>
On Fri, Jan 11, 2013 at 10:43:39AM -0800, Hilco Wijbenga wrote:
> > 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.
It depends on who you are. If you are the person running send-email,
then the symptom is your confusion. If you are somebody else, the
symptom is somebody else sending out a bogus email. That patch fixes
only the latter. :)
More seriously, I agree that re-wording the question is a reasonable
thing to do. I do not use send-email, either, so I don't have a strong
opinion on it. The suggestions you made:
> How about "What Message-ID to use as In-Reply-To for the first email?"
> or "Provide the Message-ID to use as In-Reply-To for the first
> email:".
seem fine to me. Maybe somebody who has been confused by it can offer
more. At any rate, patches welcome.
-Peff
^ permalink raw reply
* [PATCH v5] git-completion.bash: add support for path completion
From: Manlio Perillo @ 2013-01-11 18:48 UTC (permalink / raw)
To: git; +Cc: gitster, szeder, felipe.contreras, peff, Manlio Perillo
The git-completion.bash script did not implemented full, git aware,
support to complete paths, for git commands that operate on files within
the current working directory or the index.
As an example:
git add <TAB>
will suggest all files in the current working directory, including
ignored files and files that have not been modified.
Support path completion, for git commands where the non-option arguments
always refer to paths within the current working directory or the index,
as follows:
* the path completion for the "git rm" and "git ls-files"
commands will suggest all cached files.
* the path completion for the "git add" command will suggest all
untracked and modified files. Ignored files are excluded.
* the path completion for the "git clean" command will suggest all
untracked files. Ignored files are excluded.
* the path completion for the "git mv" command will suggest all cached
files when expanding the first argument, and all untracked and cached
files for subsequent arguments. In the latter case, empty directories
are included and ignored files are excluded.
* the path completion for the "git commit" command will suggest all
files that have been modified from the HEAD, if HEAD exists, otherwise
it will suggest all cached files.
For all affected commands, completion will always stop at directory
boundary. Only standard ignored files are excluded, using the
--exclude-standard option of the ls-files command.
When using a recent Bash version, Git path completion will be the same
as builtin file completion, e.g.
git add contrib/
will suggest relative file names.
Signed-off-by: Manlio Perillo <manlio.perillo@gmail.com>
---
Changes:
* Applied Junio patch to fix completion inside a subdirectory.
* Quoted the hopefully last incorrectly unquoted variable.
* Fixed coding style (removed stdout file descriptor in shell
redirection, since it is redundant).
* Fixed regression in path completion, when using non canonicalized
or absolute path names.
The problem has been solved making sure to chdir to the specified
directory before executing ls-files and diff-index commands.
The only issue is that there is no tilde expansion, but this is
harmless, since default bash completion will be used (the old
behaviour).
* Improved path completion when the new compopt builtin is available
(Bash >= 4.x).
Now git paths completion is done in exactly the same way as Bash
builtin filenames completion.
* Updated the zsh compatibility code to use the improved path
completion support
* Fixed incorrect git mv arguments count used to check the first
path to be renamed.
When options are used (unless they are git main options), -- is
required to separate options from non options arguments.
It is harmless to not use --; in this case bash will suggest
untracked files and directories for the first argument.
XXX: should I add this implementation note in the commit message?
* Make sure to sort ls-files and diff-index filtered output before
removing duplicate directories.
* Merged master.
Please note that before merging this patch in next, we need to update the
zsh and tcsh completion scripts.
I have the changes ready, but I will post them later since both scripts
needs more patches (I have posted an informal patch for zsh, and changes
to tcsh should be in pu, but I need to test them).
contrib/completion/git-completion.bash | 250 ++++++++++++++++++++++++++++++---
1 file changed, 234 insertions(+), 16 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index a4c48e1..51b8b3b 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -13,6 +13,7 @@
# *) .git/remotes file names
# *) git 'subcommands'
# *) tree paths within 'ref:path/to/file' expressions
+# *) file paths within current working directory and index
# *) common --long-options
#
# To use these routines:
@@ -233,6 +234,118 @@ __gitcomp_nl ()
COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$1" -- "${3-$cur}"))
}
+# Generates completion reply with compgen from newline-separated possible
+# completion filenames.
+# It accepts 1 to 3 arguments:
+# 1: List of possible completion filenames, separated by a single newline.
+# 2: A directory prefix to be added to each possible completion filename
+# (optional).
+# 3: Generate possible completion matches for this word (optional).
+__gitcomp_file ()
+{
+ local IFS=$'\n'
+
+ # XXX does not work when the directory prefix contains a tilde,
+ # since tilde expansion is not applied.
+ # This means that COMPREPLY will be empty and Bash default
+ # completion will be used.
+ COMPREPLY=($(compgen -P "${2-}" -W "$1" -- "${3-$cur}"))
+
+ # Tell Bash that compspec generates filenames.
+ compopt -o filenames 2>/dev/null
+}
+
+__git_index_file_list_filter_compat ()
+{
+ local path
+
+ while read -r path; do
+ case "$path" in
+ ?*/*) echo "${path%%/*}/" ;;
+ *) echo "$path" ;;
+ esac
+ done
+}
+
+__git_index_file_list_filter_bash ()
+{
+ local path
+
+ while read -r path; do
+ case "$path" in
+ ?*/*)
+ # XXX if we append a slash to directory names when using
+ # `compopt -o filenames`, Bash will append another slash.
+ # This is pretty stupid, and this the reason why we have to
+ # define a compatible version for this function.
+ echo "${path%%/*}" ;;
+ *)
+ echo "$path" ;;
+ esac
+ done
+}
+
+# Process path list returned by "ls-files" and "diff-index --name-only"
+# commands, in order to list only file names relative to a specified
+# directory, and append a slash to directory names.
+__git_index_file_list_filter ()
+{
+ # Default to Bash >= 4.x
+ __git_index_file_list_filter_bash
+}
+
+# Execute git ls-files, returning paths relative to the directory
+# specified in the first argument, and using the options specified in
+# the second argument.
+__git_ls_files_helper ()
+{
+ # NOTE: $2 is not quoted in order to support multiple options
+ cd "$1" && git ls-files --exclude-standard $2
+} 2>/dev/null
+
+
+# Execute git diff-index, returning paths relative to the directory
+# specified in the first argument, and using the tree object id
+# specified in the second argument.
+__git_diff_index_helper ()
+{
+ cd "$1" && git diff-index --name-only --relative "$2"
+} 2>/dev/null
+
+# __git_index_files accepts 1 or 2 arguments:
+# 1: Options to pass to ls-files (required).
+# Supported options are --cached, --modified, --deleted, --others,
+# and --directory.
+# 2: A directory path (optional).
+# If provided, only files within the specified directory are listed.
+# Sub directories are never recursed. Path must have a trailing
+# slash.
+__git_index_files ()
+{
+ local dir="$(__gitdir)" root="${2-.}"
+
+ if [ -d "$dir" ]; then
+ __git_ls_files_helper "$root" "$1" | __git_index_file_list_filter |
+ sort | uniq
+ fi
+}
+
+# __git_diff_index_files accepts 1 or 2 arguments:
+# 1) The id of a tree object.
+# 2) A directory path (optional).
+# If provided, only files within the specified directory are listed.
+# Sub directories are never recursed. Path must have a trailing
+# slash.
+__git_diff_index_files ()
+{
+ local dir="$(__gitdir)" root="${2-.}"
+
+ if [ -d "$dir" ]; then
+ __git_diff_index_helper "$root" "$1" | __git_index_file_list_filter |
+ sort | uniq
+ fi
+}
+
__git_heads ()
{
local dir="$(__gitdir)"
@@ -430,6 +543,46 @@ __git_complete_revlist_file ()
}
+# __git_complete_index_file requires 1 argument: the options to pass to
+# ls-file
+__git_complete_index_file ()
+{
+ local pfx cur_="$cur"
+
+ case "$cur_" in
+ ?*/*)
+ pfx="${cur_%/*}"
+ cur_="${cur_##*/}"
+ pfx="${pfx}/"
+
+ __gitcomp_file "$(__git_index_files "$1" "$pfx")" "$pfx" "$cur_"
+ ;;
+ *)
+ __gitcomp_file "$(__git_index_files "$1")" "" "$cur_"
+ ;;
+ esac
+}
+
+# __git_complete_diff_index_file requires 1 argument: the id of a tree
+# object
+__git_complete_diff_index_file ()
+{
+ local pfx cur_="$cur"
+
+ case "$cur_" in
+ ?*/*)
+ pfx="${cur_%/*}"
+ cur_="${cur_##*/}"
+ pfx="${pfx}/"
+
+ __gitcomp_file "$(__git_diff_index_files "$1" "$pfx")" "$pfx" "$cur_"
+ ;;
+ *)
+ __gitcomp_file "$(__git_diff_index_files "$1")" "" "$cur_"
+ ;;
+ esac
+}
+
__git_complete_file ()
{
__git_complete_revlist_file
@@ -722,6 +875,43 @@ __git_has_doubledash ()
return 1
}
+# Try to count non option arguments passed on the command line for the
+# specified git command.
+# When options are used, it is necessary to use the special -- option to
+# tell the implementation were non option arguments begin.
+# XXX this can not be improved, since options can appear everywhere, as
+# an example:
+# git mv x -n y
+#
+# __git_count_arguments requires 1 argument: the git command executed.
+__git_count_arguments ()
+{
+ local word i c=0
+
+ # Skip "git" (first argument)
+ for ((i=1; i < ${#words[@]}; i++)); do
+ word="${words[i]}"
+
+ case "$word" in
+ --)
+ # Good; we can assume that the following are only non
+ # option arguments.
+ ((c = 0))
+ ;;
+ "$1")
+ # Skip the specified git command and discard git
+ # main options
+ ((c = 0))
+ ;;
+ ?*)
+ ((c++))
+ ;;
+ esac
+ done
+
+ printf "%d" $c
+}
+
__git_whitespacelist="nowarn warn error error-all fix"
_git_am ()
@@ -770,8 +960,6 @@ _git_apply ()
_git_add ()
{
- __git_has_doubledash && return
-
case "$cur" in
--*)
__gitcomp "
@@ -780,7 +968,9 @@ _git_add ()
"
return
esac
- COMPREPLY=()
+
+ # XXX should we check for --update and --all options ?
+ __git_complete_index_file "--others --modified"
}
_git_archive ()
@@ -930,15 +1120,15 @@ _git_cherry_pick ()
_git_clean ()
{
- __git_has_doubledash && return
-
case "$cur" in
--*)
__gitcomp "--dry-run --quiet"
return
;;
esac
- COMPREPLY=()
+
+ # XXX should we check for -x option ?
+ __git_complete_index_file "--others"
}
_git_clone ()
@@ -969,8 +1159,6 @@ _git_clone ()
_git_commit ()
{
- __git_has_doubledash && return
-
case "$prev" in
-c|-C)
__gitcomp_nl "$(__git_refs)" "" "${cur}"
@@ -1005,7 +1193,13 @@ _git_commit ()
"
return
esac
- COMPREPLY=()
+
+ if git rev-parse --verify --quiet HEAD >/dev/null; then
+ __git_complete_diff_index_file "HEAD"
+ else
+ # This is the first commit
+ __git_complete_index_file "--cached"
+ fi
}
_git_describe ()
@@ -1223,8 +1417,6 @@ _git_init ()
_git_ls_files ()
{
- __git_has_doubledash && return
-
case "$cur" in
--*)
__gitcomp "--cached --deleted --modified --others --ignored
@@ -1237,7 +1429,10 @@ _git_ls_files ()
return
;;
esac
- COMPREPLY=()
+
+ # XXX ignore options like --modified and always suggest all cached
+ # files.
+ __git_complete_index_file "--cached"
}
_git_ls_remote ()
@@ -1369,7 +1564,14 @@ _git_mv ()
return
;;
esac
- COMPREPLY=()
+
+ if [ $(__git_count_arguments "mv") -gt 0 ]; then
+ # We need to show both cached and untracked files (including
+ # empty directories) since this may not be the last argument.
+ __git_complete_index_file "--cached --others --directory"
+ else
+ __git_complete_index_file "--cached"
+ fi
}
_git_name_rev ()
@@ -2075,15 +2277,14 @@ _git_revert ()
_git_rm ()
{
- __git_has_doubledash && return
-
case "$cur" in
--*)
__gitcomp "--cached --dry-run --ignore-unmatch --quiet"
return
;;
esac
- COMPREPLY=()
+
+ __git_complete_index_file "--cached"
}
_git_shortlog ()
@@ -2448,6 +2649,15 @@ if [[ -n ${ZSH_VERSION-} ]]; then
compadd -Q -S "${4- }" -p "${2-}" -- ${=1} && _ret=0
}
+ __gitcomp_file ()
+ {
+ emulate -L zsh
+
+ local IFS=$'\n'
+ compset -P '*[=:]'
+ compadd -Q -p "${2-}" -f -- ${=1} && _ret=0
+ }
+
__git_zsh_helper ()
{
emulate -L ksh
@@ -2469,6 +2679,14 @@ if [[ -n ${ZSH_VERSION-} ]]; then
compdef _git git gitk
return
+elif [[ -n ${BASH_VERSION-} ]]; then
+ if ((${BASH_VERSINFO[0]} < 4)); then
+ # compopt is not supported
+ __git_index_file_list_filter ()
+ {
+ __git_index_file_list_filter_compat
+ }
+ fi
fi
__git_func_wrap ()
--
1.8.1.rc1.31.ga3c84da
^ permalink raw reply related
* 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
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox