* Re: How do gmail users try out patches from this list?
From: Nguyen Thai Ngoc Duy @ 2009-08-12 13:43 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Wesley J. Landaker, Nicolas Sebrecht, skillzero, git
In-Reply-To: <4A827BF3.8080208@drmicha.warpmail.net>
On 8/12/09, Michael J Gruber <git@drmicha.warpmail.net> wrote:
> I guess for them (webmail users) it would be better if we attached
> patches, but we don't do that here. In any case, our list is mirrored on
> gmane, and you can use the interface there. For example, you get the
> first message in this thread using the gmane id or the message id like this:
>
> http://article.gmane.org/gmane.comp.version-control.git/125591
> http://mid.gmane.org/2729632a0908111343v73fa475fqb6353dcf2f718101@mail.gmail.com
>
> If you add /raw to those URLs you get the original message so that you
> can happily wget/curl/browse and save away.
even better http://download.gmane.org/gmane.comp.version-control.git/N/M
[1] (where N and M is the gmane message ID range) gives you mbox
format of a series of consecutive messages.
[1] http://gmane.org/export.php
--
Duy
^ permalink raw reply
* Re: [PATCH] Fix typos in git-remote.txt and git-symbolic-ref.txt
From: Thomas Rast @ 2009-08-12 13:40 UTC (permalink / raw)
To: Wesley J. Landaker; +Cc: Štěpán Němec, git
In-Reply-To: <200908111826.12338.wjl@icecavern.net>
Wesley J. Landaker wrote:
> On Tuesday 11 August 2009 01:09:32 Thomas Rast wrote:
> > instances of `--
[...]
> Obviously we want consistency, but--as this sentence shows--there is a
> difference between an em-dash and a command-line option like --version.
Note the backtick. This is solely about `--options` vs. `\--options`,
and I highly doubt anyone would want their em-dashes to be typeset in
fixed-width font (that's what the backticks do).
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* [PATCH] Re: [TRIVIAL] Documentation: merge: one <remote> is required
From: Nicolas Sebrecht @ 2009-08-12 13:29 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Junio C Hamano, Paul Bolle, Nicolas Sebrecht, git
In-Reply-To: <m31vnhpc5v.fsf@localhost.localdomain>
The 12/08/09, Jakub Narebski wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > After you brought up this "one or more", I re-read the docs your patches
> > touched, thinking that the author might have meant 'zero or more of A'
> > with these '<A>...' notation.
> >
> > And I realized that they made perfect sense.
This was my initial thought but...
> I would have thought that it makes more sense to have
>
> <something>...
>
> for one or more, and
>
> [<something>... ]
>
> for zero or more (optional one or more).
...their sense is what Posix states:
Ellipses ( "..." ) are used to denote that one or more occurrences of an
operand are allowed. When an option or an operand followed by ellipses
is enclosed in brackets, zero or more options or operands can be
specified. The form:
utility_name [-g option_argument]...[operand...]
http://www.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html#tag_12_01
(& 12.1.9)
--
Nicolas Sebrecht
^ permalink raw reply
* Re: How do gmail users try out patches from this list?
From: Alex Riesen @ 2009-08-12 12:51 UTC (permalink / raw)
To: skillzero; +Cc: git
In-Reply-To: <2729632a0908111343v73fa475fqb6353dcf2f718101@mail.gmail.com>
On Tue, Aug 11, 2009 at 22:43, <skillzero@gmail.com> wrote:
> Sorry if this is dumb question, but I didn't see any good info in my searches.
>
> How do gmail users normally apply patches that come through the list?
> Do you just manually copy and paste the email to patch files and use
> git apply? Do you use a tool to export to mbox files and use git am?
>
> I've been just doing it manually via copy and paste, but it's kinda tedious.
"Show original", save the page into a file (it's text/plain), remove
the first line
of spaces, and do "git am" on the file. Works every time.
^ permalink raw reply
* Re: git index: how does it work?
From: Shaun Cutts @ 2009-08-12 11:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7hxi9m4k.fsf@alter.siamese.dyndns.org>
Junio,
Your advice was very helpful.
Digging in, however, I find I still am in the dark on one point: how
does the index track renamed files, and how to query it for
information about them?
For instance, if I add a 5th step to the sequence:
5) git mv foo.c bar.c
Then I am told by "git status" that the file is renamed, but I can't
seem to elict this info using "git ls-files". Under some circumstances
even "git status" lists a new and deleted file after a rename.
Are renames being tracked by the index, and is there a more basic
interface than "status" to query about them?
Thanks for any help,
--- Shaun
On Aug 5, 2009, at 8:00 PM, Junio C Hamano wrote:
> Shaun Cutts <shaun@cuttshome.net> writes:
>
>> I am wondering if someone could explain and/or point me to an
>> explanation of how
>> the git index works.
>>
>> For instance, suppose I have a tracked file: "foo.c"
>>
>> 1) [I modify "foo.c"]
>> 2) git add foo.c
>> 3) [modify again]
>> 4) git commit -m "blah blah"
>>
>> Since I don't include the "-a" switch, the version I added on step
>> 2 is
>> committed. But how does the index keep track of these changes? Does
>> the index
>> file actually contain the hunks of "foo.c" that have been modified?
>> Or is there
>> a "temporary" blob created, which the index points to?
>
> Step 2 hashes foo.c and creates a blob object and registers it to the
> index. Step 4 writes out the index as a tree and makes a commit out
> of
> it.
>
> Running this sequence might be instructive.
>
> 1$ edit foo.c
> 2$ git add foo.c
> 2a$ git ls-files -s foo.c
> 2b$ git diff foo.c
> 2c$ git diff --cached foo.c
> 3$ edit foo.c
> 3a$ git ls-files -s foo.c
> 3b$ git diff foo.c
> 3c$ git diff --cached foo.c
> 4$ git commit -m 'half-edit of foo.c'
> 4a$ git ls-files -s foo.c
> 4b$ git ls-tree HEAD foo.c
> 4c$ git diff foo.c
> 4d$ git diff --cached foo.c
>
> - 2a shows the actual blob object that was created out of foo.c in
> step 2.
>
> - 2b shows the difference between that blob (now in the index) and
> foo.c,
> which should be empty.
>
> - 2c shows the difference between the HEAD commit and the index, which
> should show your edit in step 1.
>
> - 3a shows the blob in the index; you haven't added, so it should show
> the same as 2a.
>
> - 3b shows the difference between the index and foo.c, which should
> show
> the edit in step 3.
>
> - 3c shows the difference between the HEAD commit and the index, which
> should show your edit in step 1.
>
> - 4a shows the blob in the index; you haven't added, so it should show
> the same as 2a.
>
> - 4b shows the blob in the committed tree and the blob object should
> be
> identical to 2a.
>
> - 4c shows the difference between the index and foo.c, which should
> show
> the edit in step 3.
>
> - 4d shows the difference between the HEAD commit and the index, which
> should now be empty.
>
^ permalink raw reply
* Re: [PATCH] Re: [TRIVIAL] Documentation: merge: one <remote> is required
From: Paul Bolle @ 2009-08-12 10:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nicolas Sebrecht, git
In-Reply-To: <7vy6ppbvdf.fsf@alter.siamese.dyndns.org>
On Tue, 2009-08-11 at 19:48 -0700, Junio C Hamano wrote:
> Paul Bolle <pebolle@tiscali.nl> writes:
> > No, since "<remote>..." means one or more instances of the "<remote>"
> > option.
>
> Does it really?
>
> After you brought up this "one or more", I re-read the docs your patches
> touched, thinking that the author might have meant 'zero or more of A'
> with these '<A>...' notation.
>
> And I realized that they made perfect sense.
>
> In general, you can write:
>
> <command> ...
>
> and read this as "The <command> can be followed by nothing or something
> (zero or more) of unspecified kind". If <command> takes only one type of
> zero or more things, you can _clarify the ellipses_ by prefixing them with
> what kind of "stuff" you are talking about:
>
> <command> <remote>...
>
> and read this as "The <command> can be followed by nothing or something
> (zero or more) of <remote>s".
>
> On the other hand, you can also say (note that the ellipses stand on their
> own and are not associated with <remote>):
>
> <command> <remote> ...
>
> and read this as "It takes one <remote> followed by nothing or something
> (zero or more) of unspecified kind".
It is (now) clear to me that in these two documents the author(s) meant
"zero or more". Still, I find the "one or more" meaning more obvious.
The (GNU) manpages of "cp", "mv", "rm", and "ls" use the "one or more"
meaning. (Note that the explanation quoted above can easily be rewritten
with for "one or more" meaning and still make sense.)
Anyway, I now see that "zero or more" is used quite a lot in git's
manpages. But, that meaning doesn't fit so well with the
"[<command>...]" syntax that is also used a lot in these manpages. (I
find "optionally one or more of <command>" more obvious as otherwise
"<command>..." and "[<command>...]" are basically identical.)
Confusingly, as far as I can see, the manpages of the following commands
seem to use the "one or more" meaning:
git merge-base
git mv
git name-rev
git rm
git send-email
git tag -d
git tag -v
git verify-tag
("git mv" uses both meanings in its synopsis. The two "git tag"
invocations seem to do nothing with zero arguments and do not return an
error.)
If the above commands really use the "one or more" meaning, that would
mean both versions are used in the documentation. I'd say it would be
better to stick to one meaning throughout the manpages.
Paul Bolle
^ permalink raw reply
* Re: [PATCH] Re: [TRIVIAL] Documentation: merge: one <remote> is required
From: Jakub Narebski @ 2009-08-12 10:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Paul Bolle, Nicolas Sebrecht, git
In-Reply-To: <7vy6ppbvdf.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Paul Bolle <pebolle@tiscali.nl> writes:
>
>>> Shoudn't be
>>>
>>> [-m <msg>] <remote> [<remote>...]
>>
>> No, since "<remote>..." means one or more instances of the "<remote>"
>> option.
>
> Does it really?
>
> After you brought up this "one or more", I re-read the docs your patches
> touched, thinking that the author might have meant 'zero or more of A'
> with these '<A>...' notation.
>
> And I realized that they made perfect sense.
>
> In general, you can write:
>
> <command> ...
>
> and read this as "The <command> can be followed by nothing or something
> (zero or more) of unspecified kind". If <command> takes only one type of
> zero or more things, you can _clarify the ellipses_ by prefixing them with
> what kind of "stuff" you are talking about:
>
> <command> <remote>...
>
> and read this as "The <command> can be followed by nothing or something
> (zero or more) of <remote>s".
I would have thought that it makes more sense to have
<something>...
for one or more, and
[<something>... ]
for zero or more (optional one or more).
Documentation/DocumentationLanguage, anyone? ;-))))
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [RFC PATCH v3 8/8] --sparse for porcelains
From: Nguyen Thai Ngoc Duy @ 2009-08-12 10:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Johannes Schindelin
In-Reply-To: <7v3a7xa6e5.fsf@alter.siamese.dyndns.org>
2009/8/12 Junio C Hamano <gitster@pobox.com>:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> @@ -594,6 +596,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
>> OPT_BOOLEAN('m', "merge", &opts.merge, "merge"),
>> OPT_STRING(0, "conflict", &conflict_style, "style",
>> "conflict style (merge or diff3)"),
>> + OPT_SET_INT(0, "sparse", &opts.apply_sparse,
>> + "apply sparse checkout filter", 1),
>
> Shouldn't this be BOOLEAN not INT, i.e. "--[no-]sparse"? That way, you
> could enable it by simply the presense of $GIT_DIR/info/sparse.
This patch was written carelessly. I wanted to have something to test.
If you agree on option name "--sparse" then yes BOOLEAN is better.
> It could also require core.sparseworktree configuration set to true if we
> are really paranoid, but without the actual sparse specification file
> flipping that configuration to true would not be useful anyway, so in
> practice, giving --sparse-work-tree option to these Porcelain commands
> would be no-op, but --no-sparse-work-tree option would be useful to
> ignore $GIT_DIR/info/sparse and populate the work tree fully.
>
> Or am I missing something?
Sounds good (and --sparse-work-tree is apparently better than
--sparse). So let's enable it by default, add --no-sparse-work-tree to
disable it and wait until some one complains, then we'll add
core.sparseworktree. I think core.sparseworktree can also be used to
specify what spec file to be used instead of the default
.git/info/sparse, if users like to switch among some well-defined spec
files.
--
Duy
^ permalink raw reply
* Re: RFC for 1.7: Do not checkout -b master origin/master on clone
From: Eric Wong @ 2009-08-12 9:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael J Gruber, Git Mailing List
In-Reply-To: <7v8whpda2l.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
> > - Why does git svn set up a local branch with an svn upstream which is
> > determined by latest svn commit at the time of the first git svn fetch?
>
> I do not have comments on design decisions in git-svn, other than trusting
> that Eric would exercise good design tastes to make things coherent with
> the git native workflow when the consistency makes sense.
Using 'trunk' (if available) as the master on new fetches does probably
make more sense based on the complaints I've gotten. At the time,
several projects I cared about did almost all their work in branches and
trunk would go untouched for months.... These days most projects I care
about are already using git :)
--
Eric Wong
^ permalink raw reply
* Re: [RFC PATCH v3 8/8] --sparse for porcelains
From: Nguyen Thai Ngoc Duy @ 2009-08-12 9:53 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git, Johannes Schindelin, Junio C Hamano
In-Reply-To: <4A826FD4.5080201@viscovery.net>
2009/8/12 Johannes Sixt <j.sixt@viscovery.net>:
> BTW, the name .git/info/sparse is perhaps a bit too technical in the sense
> that only git developers know that this feature runs under the name
> "sparse checkout". Perhaps it should be named
>
> .git/info/indexonly
> .git/info/nocheckout
>
> or so.
I did not like the name "sparse" either. Another option is
.git/info/assume-unchanged.
--
Duy
^ permalink raw reply
* Re: [PATCH] svn: Add && to t9107-git-svn-migrarte.sh
From: Eric Wong @ 2009-08-12 9:49 UTC (permalink / raw)
To: Adam Brewster; +Cc: git
In-Reply-To: <1250046867-13655-1-git-send-email-adambrewster@gmail.com>
Adam Brewster <adambrewster@gmail.com> wrote:
> Eric,
>
> Any thoughts on the other patch?
>
> Am I close or is there a better way to go about this?
>
> I didn't really know what to do with .git/svn/*. The easy answer is
>
> mkdir -p $GIT_DIR/svn/refs/remotes &&
> mv $GIT_DIR/svn/* $GIT_DIR/svn/refs/remotes
That should be acceptable. I don't think it's worth the trouble to
force the majority of existing repos to move over to the new layout
(in case the user wants to switch between multiple versions of git).
> From the comments in the Migration module, it seems like that's frowned
> upon, so I came up with looking for .rev_map (or .rev_db) in both
> locations (.git/svn/git-svn and .git/svn/refs/remotes/svn) and letting
> it stay in whichever location it already exists. (The next email has a
> slightly improved version of the patch.)
>
> This solution is particularly inelegant in it's handling of
> unhandled.log, but as far as I know that file is unused.
I'd still rather avoid creating unhandled.log in two places and
having an unused set of directories.
I think the following change on top of yours would make most
sense:
diff --git a/git-svn.perl b/git-svn.perl
index cafd7fe..1117f28 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -3325,8 +3325,18 @@ sub _new {
}
$_[1] = $repo_id;
my $dir = "$ENV{GIT_DIR}/svn/$ref_id";
+
+ # Older repos imported by us used $GIT_DIR/svn/foo instead of
+ # $GIT_DIR/svn/refs/remotes/foo when tracking refs/remotes/foo
+ if ($ref_id =~ m{^refs/remotes/(.*)}) {
+ my $old_dir = "$ENV{GIT_DIR}/svn/$1";
+ if (-d $old_dir && ! -d $dir) {
+ $dir = $old_dir;
+ }
+ }
+
$_[3] = $path = '' unless (defined $path);
- mkpath(["$ENV{GIT_DIR}/svn/$ref_id"]);
+ mkpath([$dir]);
bless {
ref_id => $ref_id, dir => $dir, index => "$dir/index",
path => $path, config => "$ENV{GIT_DIR}/svn/config",
@@ -3355,16 +3365,7 @@ sub rev_db_path {
sub map_path {
my ($self, $uuid) = @_;
$uuid ||= $self->ra_uuid;
- my $map_path = "$self->{map_root}.$uuid";
- return $map_path if ( -f $map_path );
-
- my $db_path = $map_path;
- $db_path =~ s/\.rev_map/.rev_db/;
- return $map_path if ( -f $db_path );
-
- return $1 if ($map_path =~ m(refs/remotes/(.*)) && -f $1);
- return $1 if ($db_path =~ m(refs/remotes/(.*)) && -f $1);
- return $map_path;
+ "$self->{map_root}.$uuid";
}
sub uri_encode {
----
The map_path() changes you originally made didn't work, either, since
the -f $1 never took GIT_DIR or GIT_DIR/svn into account.
> There's also the problem of what to do if someone has a ref called
> ref/remotes/refs/remotes/..., but that seems unlikely enough to not
> cause concern.
I concur. There's always cases where things will be ambiguous
and cases where a user is just asking for trouble :)
I think the below is a good enough test case to for compatibility
against existing repos. Let me know what you think, thanks!
diff --git a/t/t9144-git-svn-old-rev_map.sh b/t/t9144-git-svn-old-rev_map.sh
new file mode 100755
index 0000000..7600a35
--- /dev/null
+++ b/t/t9144-git-svn-old-rev_map.sh
@@ -0,0 +1,31 @@
+#!/bin/sh
+#
+# Copyright (c) 2009 Eric Wong
+
+test_description='git svn old rev_map preservd'
+. ./lib-git-svn.sh
+
+test_expect_success 'setup test repository with old layout' '
+ mkdir i &&
+ (cd i && > a) &&
+ svn_cmd import -m- i "$svnrepo" &&
+ git svn init "$svnrepo" &&
+ git svn fetch &&
+ test -d .git/svn/refs/remotes/git-svn/ &&
+ ! test -e .git/svn/git-svn/ &&
+ mv .git/svn/refs/remotes/git-svn .git/svn/ &&
+ rm -r .git/svn/refs
+'
+
+test_expect_success 'old layout continues to work' '
+ svn_cmd import -m- i "$svnrepo/b" &&
+ git svn rebase &&
+ echo a >> b/a &&
+ git add b/a &&
+ git commit -m- -a &&
+ git svn dcommit &&
+ ! test -d .git/svn/refs/ &&
+ test -e .git/svn/git-svn/
+'
+
+test_done
--
Eric Wong
^ permalink raw reply related
* Re: [PATCH 5/8] Add a config option for remotes to specify a foreign vcs
From: Jakub Narebski @ 2009-08-12 9:33 UTC (permalink / raw)
To: Jeff King
Cc: Johannes Schindelin, Bert Wesarg, Junio C Hamano, Daniel Barkalow,
git, Brian Gernhardt
In-Reply-To: <20090812074521.GD15152@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> 1. Is there some other syntax that _doesn't_ have this breakage
> but that similarly helps the "vast majority of Git users".
Well, proposed possible syntax was:
1. <vcs>:<repository location>
e.g.:
svn:http://svn.example.com/project
but
host:path/to/repo
2. <vcs>::<repository location>
e.g.
svn::http://svn.example.com/project
3. <vcs>+<repository location>
e.g.
svn+http://svn.example.com/project
but
http+svn://svn.example.com/project
svn+path/to/repo
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: Problems with filters and git status - reproduction steps
From: Peter Krefting @ 2009-08-12 9:25 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Johannes Sixt, Git List
In-Reply-To: <4A828368.5010206@drmicha.warpmail.net>
Michael J Gruber:
> I get "Changed but not updated" already here!
With git 1.5.6.5, I do get that. With git 1.6.4, I seem to only get it after
the last step.
I have 1.5.6.5 on the server with the master repo, and am running 1.6.4 on
the client (although when I ran the recipe through a shell script, I got the
unclean status earlier, which made me think it ran an earlier version I have
installed in another directory).
> Do you really want the date in the checked-in version of the file?
Yes. This way, the checked out copy that makes my web server can do its job
without requiring any of the filters being installed. And the files in
history are marked as necessary.
> I would assume otherwise. Then your clean filter should really be the
> smudge filter, and you would need a clean filter to go with it (remove the
> date and restore the keyword).
The problem with that approach is that the "smudge" filter does not have
access to the file name, and so can not look up the last change date of the
file it is re-writing. And I want the last time the file was *changed*, not
the time it was checked out.
Also, I imported my entire CVS history with keywords expanded to allow for
this (and "checked out" the Git-generated tree over my CVS check-out to not
have Git update all the time-stamps).
--
\\// Peter - http://www.softwolves.pp.se/
^ permalink raw reply
* Re: [RFCv3 2/4] Add Python support library for CVS remote helper
From: Johan Herland @ 2009-08-12 9:08 UTC (permalink / raw)
To: David Aguilar; +Cc: git, barkalow, gitster, Johannes.Schindelin
In-Reply-To: <20090812021017.GB62301@gmail.com>
First, thank you very much for the review. It is very helpful, and I really
appreciate it.
On Wednesday 12 August 2009, David Aguilar wrote:
> On Wed, Aug 12, 2009 at 02:13:49AM +0200, Johan Herland wrote:
> > This patch introduces a Python package called "git_remote_cvs"
> > containing the building blocks of the CVS remote helper. The CVS remote
> > helper itself is NOT part of this patch.
>
> Interesting...
>
> > diff --git a/git_remote_cvs/changeset.py b/git_remote_cvs/changeset.py
> > new file mode 100644
> > index 0000000..27c4129
> > --- /dev/null
> > +++ b/git_remote_cvs/changeset.py
> > @@ -0,0 +1,114 @@
> > +#!/usr/bin/env python
> > +
> > +"""Functionality for collecting individual CVS revisions into
> > "changesets" +
> > +A changeset is a collection of CvsRev objects that belong together in
> > the same +"commit". This is a somewhat artificial construct on top of
> > CVS, which only +stores changes at the per-file level. Normally, CVS
> > users create several CVS +revisions simultaneously by applying the "cvs
> > commit" command to several files +with related changes. This module
> > tries to reconstruct this notion of related +revisions.
> > +"""
> > +
> > +from util import *
>
> Importing * is frowned upon in Python.
>
> It's much easier to see where things are coming from if you
> 'import util' and use the namespaced util.foo() way of accessing
> the functions.
I'd rather do "from util import X Y Z", as the util stuff is used all over
the place.
> Furthermore, you're going to want to use absolute imports.
> Anyone can create 'util.py' and blindly importing 'util' is
> asking for trouble.
>
> Instead use:
> from git_remote_cvs import util
I thought the python import rules specified that the current package was
consulted first, and therefore the 'util' package would always come from the
current package. However, I must confess that I don't know these rules very
well, so I'll take your word for it and use absolute imports instead.
> > +class Changeset (object):
> > + """Encapsulate a single changeset/commit"""
>
> I think it reads better as Changeset(object)
> (drop the spaces before the parens).
>
> That applies to the rest of this patch as well.
Ok. Will change.
> This also had me wondering about the following:
> git uses tabs for indentation
>
> BUT, the python convention is to use 4-space indents ala PEP-8
> http://www.python.org/dev/peps/pep-0008/
Interesting. I have (obviously) never looked at PEP 8... :)
> It might be appealing to when-in-Rome (Rome being Python) here
> and do things the python way when we code in Python.
>
> Consistency with pep8 is good if we expect to get python hackers
> to contribute to git_remote_cvs.
I see your point, but I believe that since git_remote_cvs is not an
independent project (but very much coupled to git), its allegiance is with
Git, and it should therefore follow the Git coding style. In other words, I
claim exception (2) in PEP 8
> > +
> > + __slots__ = ('revs', 'date', 'author', 'message')
>
> __slots__ is pretty esoteric in Python-land.
>
> But, if your justification is to minimize memory usage, then
> yes, this is a good thing to do.
Yes, I only use __slots__ for classes that potentially have a large number
of instances.
> > + def __init__ (self, date, author, message):
> > + self.revs = {} # dict: path -> CvsRev object
> > + self.date = date # CvsDate object
> > + self.author = author
> > + self.message = message # Lines of commit message
>
> pep8 and other parts of the git codebase recommend against
> lining up the equals signs like that. Ya, sorry for the nits
> being that they're purely stylistic.
I can't find a good rationale for this rule in PEP8 (other than Guido's
personal style), and I personally find the above much more readable
(otherwise I wouldn't go through the trouble of lining them all up...). Can
I claim exception (1) (readability)?
> > + if len(msg) > 25: msg = msg[:22] + "..." # Max 25 chars long
> > + return "<Changeset @(%s) by %s (%s) updating %i files>" % (
> > + self.date, self.author, msg, len(self.revs))
>
> Similar to the git coding style, this might be better written:
>
> ...
> if len(msg) > 25:
> msg = msg[:22] + '...' # Max 25 chars long
> ...
>
> (aka avoid single-line ifs)
>
> There's a few other instances of this in the patch as well.
Ok. Will try to eliminate single-line ifs.
> > diff --git a/git_remote_cvs/cvs.py b/git_remote_cvs/cvs.py
> > new file mode 100644
> > index 0000000..cc2e13f
> > --- /dev/null
> > +++ b/git_remote_cvs/cvs.py
> > @@ -0,0 +1,884 @@
> > [...]
> > +
> > + def enumerate (self):
> > + """Return a list of integer components in this CVS number"""
> > + return list(self.l)
>
> enumerate has special meaning in Python.
>
> items = (1, 2, 3, 4)
> for idx, item in enumerate(items):
> print idx, item
>
>
> I'm not sure if this would cause confusion...
Good point, I should probably rename this method.
> > [...]
> > + else: # revision number
> > + assert self.l[-1] > 0
>
> asserts go away when running with PYTHONOPTIMIZE.
>
> If this is really an error then we should we raise an exception
> instead?
I use asserts to verify pre/post-conditions and other invariants. I believe
that if this assert fails, it is indicative of something horribly wrong with
the code itself. However, I now see that one can also trigger this case with
bad input (e.g. CvsNum("1.2.3.0").parent()). I will keep the assert here,
but will also add some input verification to the CvsNum class.
> > + @classmethod
> > + def test (cls):
> > + assert cls("1.2.4") == cls("1.2.0.4")
>
> Hmm.. Does it make more sense to use the unittest module?
>
> e.g. self.assertEqual(foo, bar)
Probably. I'm not familiar with 'unittest', but will take a look.
> > diff --git a/git_remote_cvs/cvs_revision_map.py
> > b/git_remote_cvs/cvs_revision_map.py new file mode 100644
> > index 0000000..7d7810f
> > --- /dev/null
> > +++ b/git_remote_cvs/cvs_revision_map.py
> > @@ -0,0 +1,362 @@
> > +#!/usr/bin/env python
> > +
> > +"""Functionality for mapping CVS revisions to associated
> > metainformation""" +
> > +from util import *
> > +from cvs import CvsNum, CvsDate
> > +from git import GitFICommit, GitFastImport, GitObjectFetcher
>
> We definitely need absolute imports here.
>
> 'import git' could find the git-python project's git module.
Ok. Will fix.
> Nonetheless, interesting stuff.
Thanks for the review!
Have fun! :)
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: Problems with filters and git status - reproduction steps
From: Michael J Gruber @ 2009-08-12 8:55 UTC (permalink / raw)
To: Peter Krefting; +Cc: Johannes Sixt, Git List
In-Reply-To: <alpine.DEB.2.00.0908120856110.30907@ds9.cixit.se>
Peter Krefting venit, vidit, dixit 12.08.2009 10:36:
> ORIGINREPO=git://git.debian.org/users/peterk/gitfilterproblem.git
> DESTINATIONREPO=gitfilterproblem-testrepo
>
> # Set up repository
> echo -- Cloning
> git clone ${ORIGINREPO} ${DESTINATIONREPO}
> cd ${DESTINATIONREPO}
>
> # Status should be clean
> echo -- After cloning, status should be clean
> git status
>
> # Set up filter
> echo -- Set up filter, status should be clean
> ./reposetup.sh
> git status
I get "Changed but not updated" already here!
Also, what's your git version? There have been some fix-ups recently
regarding the interaction between filters/textconv and assumptions of
the code about dirtiness of the worktree.
>
> # Create tracking branch
> echo -- Create tracking branch for changed file, status should be clean
> git branch --track changed-text-files origin/changed-text-files
> git status
>
> # Merge the branch
> echo -- Merge the changes from the branch, now status gets unclean
> git merge changed-text-files
> echo
> git status
> git diff
Do you really want the date in the checked-in version of the file? I
would assume otherwise. Then your clean filter should really be the
smudge filter, and you would need a clean filter to go with it (remove
the date and restore the keyword).
Michael
^ permalink raw reply
* Re: How do gmail users try out patches from this list?
From: Mike Ralphson @ 2009-08-12 8:43 UTC (permalink / raw)
To: skillzero; +Cc: Wesley J. Landaker, Nicolas Sebrecht, git, Michael J Gruber
In-Reply-To: <4A827BF3.8080208@drmicha.warpmail.net>
2009/8/11 <skillzero@gmail.com>:
> Sorry if this is dumb question, but I didn't see any good info in my searches.
>
> How do gmail users normally apply patches that come through the list?
> Do you just manually copy and paste the email to patch files and use
> git apply? Do you use a tool to export to mbox files and use git am?
>
> I've been just doing it manually via copy and paste, but it's kinda tedious.
Yep, show original, copy and paste and git apply. Personally I would
prefer to fetch changes using, oh, I don't know, some kind of dvcs
tool... 8-) It means I don't tend to build and test many patch series
until they get merged.
It's a pity there's a patchwork server for many kernel.org projects,
but not for the git mailing list 8-(
http://patchwork.kernel.org/
http://patchwork.kernel.org/help/pwclient/
http://gitster.livejournal.com/18696.html
^ permalink raw reply
* Problems with filters and git status - reproduction steps
From: Peter Krefting @ 2009-08-12 8:36 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Git List
In-Reply-To: <alpine.DEB.2.00.0908120751500.30907@ds9.cixit.se>
Peter Krefting:
> However, as I have set the "ident" attribute, Git wants to expand it itself
> and check in files with "$Id$". When I do a reset, it seems it records the
> entry as clean against a version stored with just "$Id$", but the record in
> history has an expanded "$Id$", and the entry is thus never deemed clean.
Actually, that is not the case. It seems to be the filter that causes the
problems, all by itself. I cannot seem to reproduce this *reliably*. I tried
setting up a minimal repository and a reproduction recipe, but I get
different behaviour when I perform the steps manually, and when I run it
from a shell script.
Here is the reproduction recipe:
#!/bin/bash
# Reproduction recipe for $Date$ dirty issue
ORIGINREPO=git://git.debian.org/users/peterk/gitfilterproblem.git
DESTINATIONREPO=gitfilterproblem-testrepo
# Set up repository
echo -- Cloning
git clone ${ORIGINREPO} ${DESTINATIONREPO}
cd ${DESTINATIONREPO}
# Status should be clean
echo -- After cloning, status should be clean
git status
# Set up filter
echo -- Set up filter, status should be clean
./reposetup.sh
git status
# Create tracking branch
echo -- Create tracking branch for changed file, status should be clean
git branch --track changed-text-files origin/changed-text-files
git status
# Merge the branch
echo -- Merge the changes from the branch, now status gets unclean
git merge changed-text-files
echo
git status
git diff
--
\\// Peter - http://www.softwolves.pp.se/
^ permalink raw reply
* Re: How do gmail users try out patches from this list?
From: Michael J Gruber @ 2009-08-12 8:23 UTC (permalink / raw)
To: Wesley J. Landaker; +Cc: Nicolas Sebrecht, skillzero, git
In-Reply-To: <200908111917.19267.wjl@icecavern.net>
Wesley J. Landaker venit, vidit, dixit 12.08.2009 03:17:
> On Tuesday 11 August 2009 16:14:08 Nicolas Sebrecht wrote:
>> The 11/08/09, skillzero@gmail.com wrote:
>>> Sorry if this is dumb question, but I didn't see any good info in my
>>> searches.
>>>
>>> How do gmail users normally apply patches that come through the list?
>>
>> It doesn't rely on your address mail provider but on your local email
>> workflow/MUA.
>
> I'm not in this situation, but my guess is that a lot of people use gmail
> primarily through the web interface (e.g. because of corporate firewalls or
> some other reason). Maybe someone in that situation should make an new "git
> imap-am" command? Kind of the reverse to imap-send. Just a thought. =)
Well, if they can't do imap (because of a firewall) git can't do imap...
I guess for them (webmail users) it would be better if we attached
patches, but we don't do that here. In any case, our list is mirrored on
gmane, and you can use the interface there. For example, you get the
first message in this thread using the gmane id or the message id like this:
http://article.gmane.org/gmane.comp.version-control.git/125591
http://mid.gmane.org/2729632a0908111343v73fa475fqb6353dcf2f718101@mail.gmail.com
If you add /raw to those URLs you get the original message so that you
can happily wget/curl/browse and save away.
Michael
^ permalink raw reply
* Re: fatal: bad revision 'HEAD'
From: Jeff King @ 2009-08-12 7:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Joel Mahoney, Johannes Schindelin, git
In-Reply-To: <7v7hx98otz.fsf@alter.siamese.dyndns.org>
On Wed, Aug 12, 2009 at 12:37:44AM -0700, Junio C Hamano wrote:
> But just like we twisted the definition of merge to mean "merging
> something into nothing yields that something", we could twist the
> definition of rebase to mean "rebasing nothing on top of something result
> in that something". It sort of makes sense in a twisted way.
I dunno. It doesn't seem all that twisted to me.
But like many of the "branch to be born" and "initial commit" edge cases
we have dealt with, it is not so much about somebody intentionally
triggering this as it is about doing something sane when some script
_does_ trigger it. And I think the sane thing is obvious and easy to do
here, so why not?
> * Is "rev-parse -q --verify" a safe test to guarantee that HEAD is
> unborn? Shouldn't we be checking with "symbolic-ref" or something?
I'm not sure. The test in git-checkout, for example, seems to basically
just be looking up HEAD as a commit. If it doesn't work, then the branch
is to-be-born (see switch_branches in builtin-checkout.c).
Which is more or less what's happening here (except we don't check that
the type is a commit).
With symbolic-ref, I guess we could find out what the ref is, and check
to see if _that_ exists. But I can't think of a situation where that
would be meaningfully different than just resolving HEAD. Obviously
detached HEADs come to mind, but wouldn't you then by definition not be
a branch-to-be-born, which is what this rev-parse test would tell you?
> * In such an "unborn branch" case, by definition, a non-empty index won't
> be based on whatever we are pulling down from the remote. So how about
> doing something like the following instead?
>
> if on unborn branch
> then
> if test -f "$GIT_DIR/index"
> then
> die "refusing to update; you have a non-empty index"
> fi
> else
> ... existing tests against HEAD ...
> fi
Yeah, I think that is a better idea. Do you want to tweak the patch, or
should I re-submit?
-Peff
^ permalink raw reply
* Re: [RFC/PATCH 5/6] Let transport_helper_init() decide if a remote helper program can be used
From: Jeff King @ 2009-08-12 7:46 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Johan Herland, git, gitster, benji, Johannes.Schindelin
In-Reply-To: <alpine.LNX.2.00.0908111915100.27553@iabervon.org>
On Tue, Aug 11, 2009 at 07:28:19PM -0400, Daniel Barkalow wrote:
> of 'Could not find (...) "git remote-master.kernel.org" (...)'? That
> would be certain to upset some people. I think we can assume that people's
> scheme parts of their URLs that are actually URLs don't contain '.', and
> that people with:
>
> url = master:something
>
> will append their domains if the warning gets annoying.
Keep in mind that these URLs should be usable from the command-line,
too. So it is not just appending the domain in the config, but appending
it every time you want to do a one-off pull.
-Peff
^ permalink raw reply
* Re: [PATCH 5/8] Add a config option for remotes to specify a foreign vcs
From: Jeff King @ 2009-08-12 7:45 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Bert Wesarg, Junio C Hamano, Daniel Barkalow, git,
Brian Gernhardt
In-Reply-To: <alpine.DEB.1.00.0908120128120.8306@pacific.mpi-cbg.de>
On Wed, Aug 12, 2009 at 01:53:38AM +0200, Johannes Schindelin wrote:
> > It is not actually that unreasonable. I have remotes which point to:
> >
> > vcs:git/foo.git
>
> That is still not "svn".
No, but you snipped the part where I explain how that leads me to
believe "svn" is plausible. Remember that you and I are just a
representative sample of a much larger userbase.
There is also a related question: should the the meaning of the URL rely
purely on _syntax_, or must we understand the _semantics_ of the
individual tokens? That is, given $X:$Y, does that syntactically mean
that $X _must_ be a remote helper, or must I understand what helpers git
knows about to know what it is?
I tend to think purely syntactic systems are more robust and easier to
understand. The downside is that it's less DWIM, which can often mean
more typing.
> If _I_ were to judge whether to make it convenient for computer-savvy
> people like you who would have _no_ problem diagnosing the problem (_if_
> they have the problem, having edited .ssh/config themselves!), who would
> curse briefly, and then go on fixing the problem, or in the alternative
> make it convenient for people who do not know their way around .ssh/config
> as well as you (and who happen to make up the _vast_ majority of Git users
> by now [*1*]), and who would really prefer to have an easy way to clone
> "foreign" repositories, I have _no_ problem deciding which way to go.
>
> So I'm a bastard. Big news. But I'm a pragmatic one.
You didn't quote the part of my email about how ssh:// sucks. It is not
just about having my config break, figuring it out, and fixing it. You
are losing a useful construct that I might be using on the command line.
That being said, I am not 100% opposed to the proposal. I just think it
is worth considering this breakage as a downside, and considering
1. Is there some other syntax that _doesn't_ have this breakage
but that similarly helps the "vast majority of Git users".
2. Should such a breakage follow a deprecation schedule, and if so,
what schedule?
-Peff
^ permalink raw reply
* Re: [EGIT PATCH] Provide a more JavaBeans-style 'getName' accessor for the id Signed-off-by: Alex Blewitt <alex.blewitt@gmail.com>
From: Alex Blewitt @ 2009-08-11 12:53 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: robin.rosenberg@dewire.com, git@vger.kernel.org
In-Reply-To: <20090810205907.GY1033@spearce.org>
On 10 Aug 2009, at 21:59, "Shawn O." <spearce@spearce.org> wrote:
> Alex Blewitt <alex.blewitt@gmail.com> wrote:
>> That patch was originally mailed on the 11th May. Has it taken
>> until now
>> to notice the problem, or was the other method added in the last
>> month or
>> so? If I'm to blame, I apologise but didn't note any compile time
>> issues
>> at the time.
>
> Arrgh, you are right, I lost this patch in my inbox, and in the
> interm we applied new features to RevTag which added getName there. .
>
>>> ./org/spearce/jgit/revwalk/RevTag.java:206: getName() in
>>> org.spearce.jgit.revwalk.RevTag cannot override getName() in
>>> org.spearce.jgit.lib.AnyObjectId; overridden method is final
>
> I can't apply this patch because getName() on RevTag is already
> defined with a different meaning. :-(
That sounds dangerous. We now have a .name() and a .getName() with
different semantics. Can we not change the RevTag method name to
something else so that we dont have an inconsistency?
>
Alex
^ permalink raw reply
* Re: fatal: bad revision 'HEAD'
From: Junio C Hamano @ 2009-08-12 7:37 UTC (permalink / raw)
To: Jeff King; +Cc: Joel Mahoney, Johannes Schindelin, git
In-Reply-To: <20090812032740.GA26089@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I was able to replicate your problem, but only if I set
> branch.master.rebase to "true" in my user-wide git config (i.e.,
> ~/.gitconfig). It looks like "git pull" is not capable of handling a
> rebase when you have no commits yet.
It does sound sick to store such a setting in $HOME/.gitconfig file, when
the variable is clearly per-repository. As you wrote, you can easily
trigger it with an explicit --rebase, but again, it is insane to ask
"rebase" when you clearly do not have anything.
But just like we twisted the definition of merge to mean "merging
something into nothing yields that something", we could twist the
definition of rebase to mean "rebasing nothing on top of something result
in that something". It sort of makes sense in a twisted way.
> diff --git a/git-pull.sh b/git-pull.sh
> index 0f24182..427b5c6 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -119,9 +119,15 @@ error_on_no_merge_candidates () {
> }
>
> test true = "$rebase" && {
> + if git rev-parse -q --verify HEAD >/dev/null; then
> + parent_tree=HEAD
> + else # empty tree
> + parent_tree=4b825dc642cb6eb9a060e54bf8d69288fbee4904
> + fi
> +
> git update-index --ignore-submodules --refresh &&
> git diff-files --ignore-submodules --quiet &&
> - git diff-index --ignore-submodules --cached --quiet HEAD -- ||
> + git diff-index --ignore-submodules --cached --quiet $parent_tree -- ||
> die "refusing to pull with rebase: your working tree is not up-to-date"
Two comments.
* Is "rev-parse -q --verify" a safe test to guarantee that HEAD is
unborn? Shouldn't we be checking with "symbolic-ref" or something?
* In such an "unborn branch" case, by definition, a non-empty index won't
be based on whatever we are pulling down from the remote. So how about
doing something like the following instead?
if on unborn branch
then
if test -f "$GIT_DIR/index"
then
die "refusing to update; you have a non-empty index"
fi
else
... existing tests against HEAD ...
fi
^ permalink raw reply
* Re: [RFC PATCH v3 8/8] --sparse for porcelains
From: Johannes Sixt @ 2009-08-12 7:31 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy
Cc: git, Johannes Schindelin, Junio C Hamano
In-Reply-To: <1250005446-12047-9-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy schrieb:
> This series is useless until now because no one would use read-tree to
> checkout. At least with this, you can really use/test the series.
> Porcelain design was originally "if you have .git/info/sparse,
> porcelains will use it, if you don't like that, remove
> .git/info/sparse" while plumblings have an option to
> enable/disable this feature.
>
> And I still like that behavior. How about we enable sparse checkout
> by default for porcelains and make a config option to disable it?
I would enable sparse checkout by default even for plumbing. Whether the
checkout area is sparse should always be governed by .git/info/sparse.
This way, existing scripts and aliases should automatically work in sparse
worktrees.
BTW, the name .git/info/sparse is perhaps a bit too technical in the sense
that only git developers know that this feature runs under the name
"sparse checkout". Perhaps it should be named
.git/info/indexonly
.git/info/nocheckout
or so.
-- Hannes
^ permalink raw reply
* Re: [msysGit] Re: Using VC build git
From: Marius Storm-Olsen @ 2009-08-12 7:13 UTC (permalink / raw)
To: Frank Li; +Cc: Johannes Schindelin, git, msysGit
In-Reply-To: <1976ea660908101826q26faa37ao7920d5cf9d4f53fd@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2288 bytes --]
[Please do *not* do top-posting! Both git and msysgit mailing lists
use bottom-posting]
Frank Li said the following on 11.08.2009 03:26:
> Thank you take care my patch.
> I can fix all problems.
Good! Many people want to see git build with MSVC, if only to use a
compiler better at optimizing code on Windows. (And the debugger, of
course)
> This patch is base on v1.6.4 release. My working branch is vc_build
> at git://repo.or.cz/tgit.git.
Ok. Dscho wondered why it wasn't a proper fork of the main git.git
repo, so it *should* really have been
git://repo.or.cz/git/tgit.git
^^^^ <-- Notice the fork relationship?
It would save some valuable space on the server, show relations, make
it easier to find, etc.
> That is actually prototype to approve VC can build git.
> The code style is not big problem. I will fix it.
You will experience that for the git community coding style is very
important (for good reason), so expect many rounds of rewriting your
patches, until they all shine like diamonds.
> VC build will reuse many msysgit works because msysgit really do many
> work at windows porting.
Sure, but I think it rarely will involve your patches changing the
code in MinGW at all. Try your outmost to keep your patches separated
from anything else. If in doubt, please ask us on the msysgit mailing
list, and we will guide you.
> I think the below is most important problem.
>
> 1. Where are vcbuild directory put, is it okay under contrib ?
> 2. How to handle external library, such as zlib? Can use submodule?
For point 2: If we cannot compile we don't accept it, is the basic
rule. This means that if you think the people building git with MSVC
cannot install some dependencies themselves, you need to provide the
full sources yourself, and make sure that the code compiles without
changes. So, in this case if would mean to include the sources for
zlib, and setup the vcproj to compile this code into the executable,
instead of linking with a precompiled lib.
In this case, however, I think you should rely on the developer
providing this library themselves, and not add it to the git project.
The zlib project is ~750KB of code itself..
--
.marius
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 187 bytes --]
^ 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