* [PATCH v2 2/3] t9600: fixup for new cvsimport
From: Chris Rorvick @ 2013-01-12 6:21 UTC (permalink / raw)
To: git; +Cc: Eric S. Raymond, Junio C Hamano, Chris Rorvick
In-Reply-To: <1357971703-28513-1-git-send-email-chris@rorvick.com>
cvsimport no longer supports -a (import all commits including recent ones)
and no longer uses the 'origin' branch by default for imports.
Signed-off-by: Chris Rorvick <chris@rorvick.com>
---
t/t9600-cvsimport.sh | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/t/t9600-cvsimport.sh b/t/t9600-cvsimport.sh
index 4c384ff..14f54d5 100755
--- a/t/t9600-cvsimport.sh
+++ b/t/t9600-cvsimport.sh
@@ -44,7 +44,7 @@ EOF
test_expect_success PERL 'import a trivial module' '
- git cvsimport -a -R -z 0 -C module-git module &&
+ git cvsimport -R -z 0 -C module-git module &&
test_cmp module-cvs/o_fortuna module-git/o_fortuna
'
@@ -90,8 +90,7 @@ test_expect_success PERL 'update git module' '
(cd module-git &&
git config cvsimport.trackRevisions true &&
- git cvsimport -a -z 0 module &&
- git merge origin
+ git cvsimport -z 0 module
) &&
test_cmp module-cvs/o_fortuna module-git/o_fortuna
@@ -119,8 +118,7 @@ test_expect_success PERL 'cvsimport.module config works' '
(cd module-git &&
git config cvsimport.module module &&
git config cvsimport.trackRevisions true &&
- git cvsimport -a -z0 &&
- git merge origin
+ git cvsimport -z0
) &&
test_cmp module-cvs/tick module-git/tick
@@ -140,7 +138,7 @@ test_expect_success PERL 'import from a CVS working tree' '
$CVS co -d import-from-wt module &&
(cd import-from-wt &&
git config cvsimport.trackRevisions false &&
- git cvsimport -a -z0 &&
+ git cvsimport -z0 &&
echo 1 >expect &&
git log -1 --pretty=format:%s%n >actual &&
test_cmp actual expect
--
1.8.1.rc3.335.g88a67d6
^ permalink raw reply related
* [PATCH v2 3/3] t9604: fixup for new cvsimport
From: Chris Rorvick @ 2013-01-12 6:21 UTC (permalink / raw)
To: git; +Cc: Eric S. Raymond, Junio C Hamano, Chris Rorvick
In-Reply-To: <1357971703-28513-1-git-send-email-chris@rorvick.com>
cvsps no longer writes a cache file and therefore no longer can be told
to ignore it with -x.
Signed-off-by: Chris Rorvick <chris@rorvick.com>
---
t/t9604-cvsimport-timestamps.sh | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/t/t9604-cvsimport-timestamps.sh b/t/t9604-cvsimport-timestamps.sh
index 1fd5142..b1629b6 100755
--- a/t/t9604-cvsimport-timestamps.sh
+++ b/t/t9604-cvsimport-timestamps.sh
@@ -7,8 +7,7 @@ setup_cvs_test_repository t9604
test_expect_success 'check timestamps are UTC (TZ=CST6CDT)' '
- TZ=CST6CDT git cvsimport -p"-x" -C module-1 module &&
- git cvsimport -p"-x" -C module-1 module &&
+ TZ=CST6CDT git cvsimport -C module-1 module &&
(
cd module-1 &&
git log --format="%s %ai"
@@ -42,7 +41,7 @@ test_expect_success 'check timestamps with author-specific timezones' '
user3=User Three <user3@domain.org> EST5EDT
user4=User Four <user4@domain.org> MST7MDT
EOF
- git cvsimport -p"-x" -A cvs-authors -C module-2 module &&
+ git cvsimport -A cvs-authors -C module-2 module &&
(
cd module-2 &&
git log --format="%s %ai %an"
--
1.8.1.rc3.335.g88a67d6
^ permalink raw reply related
* Re: [PATCH v2 0/3] fixup remaining cvsimport tests
From: Junio C Hamano @ 2013-01-12 6:36 UTC (permalink / raw)
To: Chris Rorvick; +Cc: git, Eric S. Raymond
In-Reply-To: <1357971703-28513-1-git-send-email-chris@rorvick.com>
Chris Rorvick <chris@rorvick.com> writes:
> Reroll w/ sign-off.
>
> Chris Rorvick (3):
> t/lib-cvs.sh: allow cvsps version 3.x.
> t9600: fixup for new cvsimport
> t9604: fixup for new cvsimport
>
> t/lib-cvs.sh | 2 +-
> t/t9600-cvsimport.sh | 10 ++++------
> t/t9604-cvsimport-timestamps.sh | 5 ++---
> 3 files changed, 7 insertions(+), 10 deletions(-)
Thanks.
I too noticed the droppage of "-a" support, which may not be a big
deal (people can drop it from their script, run cvsimport and they
can drop newer commits from the resulting Git history to emulate the
old behaviour without "-a" that attempted to find a quiescent point
if they really want to and suspect that the upstream CVS repository
was not quiescent during the import).
Likewise for "-x". You said "no longer can be told" and that is
technically true, but it is more like "no longer need to be told, as
stale cache cannot get in the way", so it is probably not a big
deal, either, for people to drop it from their script.
About the changed behaviour regarding "origin", I suspect that it is
a change for the better, but we would probably need documentation
updates to cover it (and deleted options and (mis)features) before
this topic graduates.
^ permalink raw reply
* Re: [PATCH v2 0/3] fixup remaining cvsimport tests
From: Chris Rorvick @ 2013-01-12 7:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Eric S. Raymond
In-Reply-To: <7vr4lq9acu.fsf@alter.siamese.dyndns.org>
On Sat, Jan 12, 2013 at 12:36 AM, Junio C Hamano <gitster@pobox.com> wrote:
> I too noticed the droppage of "-a" support, which may not be a big
> deal (people can drop it from their script, run cvsimport and they
> can drop newer commits from the resulting Git history to emulate the
> old behaviour without "-a" that attempted to find a quiescent point
> if they really want to and suspect that the upstream CVS repository
> was not quiescent during the import).
Is there any value to -a when fuzz is exposed (-z)? I mean this is a
functional sense. I think there is a lot of value to maintaining the
interfaces of both cvsimport and cvsps where possible.
> Likewise for "-x". You said "no longer can be told" and that is
> technically true, but it is more like "no longer need to be told, as
> stale cache cannot get in the way", so it is probably not a big
> deal, either, for people to drop it from their script.
:-) I originally wrote "need" and then changed it to be clearer on
why it was being removed.
^ permalink raw reply
* Re: [BUG] Possible bug in `remote set-url --add --push`
From: Junio C Hamano @ 2013-01-12 7:10 UTC (permalink / raw)
To: Jardel Weyrich; +Cc: git
In-Reply-To: <CAN8TAOsnX1Mr72LPa47KKXDeUZPgSHTJ6u4YpPFPrtsK7VdN+A@mail.gmail.com>
Jardel Weyrich <jweyrich@gmail.com> writes:
> I believe `remote set-url --add --push` has a bug. Performed tests
> with v1.8.0.1 and v1.8.1 (Mac OS X).
>
> Quoting the relevant part of the documentation:
>
>> set-url
>> Changes URL remote points to. Sets first URL remote points to matching regex <oldurl> (first URL if no <oldurl> is given) to <newurl>. If <oldurl> doesn’t match any URL, error occurs and nothing is changed.
>>
>> With --push, push URLs are manipulated instead of fetch URLs.
>> With --add, instead of changing some URL, new URL is added.
>> With --delete, instead of changing some URL, all URLs matching regex <url> are deleted. Trying to delete all non-push URLs is an error.
>
> Here are some steps to reproduce:
>
> 1. Show the remote URLs
>
> jweyrich@pharao:test_clone1 [* master]$ git remote -v
> origin /Volumes/sandbox/test (fetch)
> origin /Volumes/sandbox/test (push)
>
> 2. Add a new push URL for origin
>
> jweyrich@pharao:test_clone1 [* master]$ git remote set-url --add --push origin \
> /Volumes/sandbox/test_clone2
>
> 3. Check what happened
>
> jweyrich@pharao:test_clone1 [* master]$ git remote -v
> origin /Volumes/sandbox/test (fetch)
> origin /Volumes/sandbox/test_clone2 (push)
The original pushurl was replaced with the additional one, instead
of being left and the new one getting added. That looks certainly
wrong.
However, the result of applying the attached patch (either to
v1.7.12 or v1.8.1) still passes the test and I do not think it is
doing anything differently from what you described above.
What do you get from
git config -l | grep '^remote\.origin'
in steps 1. and 3. in your procedure? This question is trying to
tell if your bug is in "git remote -v" or in "git remote set-url".
-- >8 --
From 0f6cbc67db926e97707ae732b02e790b4604508e Mon Sep 17 00:00:00 2001
From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 11 Jan 2013 23:04:16 -0800
Subject: [PATCH] t5505: adding one pushurl from jweyrich
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t5505-remote.sh | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index c03ffdd..b31c5bb 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -901,6 +901,25 @@ test_expect_success 'remote set-url --push --add aaa' '
cmp expect actual
'
+test_expect_success 'remote set-url --push --add' '
+ git config remote.jweyrich.url /Volumes/sandbox/test &&
+ git config remote.jweyrich.pushurl /Volumes/sandbox/test &&
+ git config remote.jweyrich.fetch "refs/heads/*:refs/remotes/jweyrich/*" &&
+
+ added=/Volumes/sandbox/test_clone2 &&
+ {
+ git config -l | grep "^remote\.jweyrich\." &&
+ echo "remote.jweyrich.pushurl=$added"
+ } | sort >expect &&
+
+ git remote set-url --add --push jweyrich "$added" &&
+ git config -l | grep "^remote\.jweyrich\." | sort >actual &&
+
+ test_cmp expect actual &&
+
+ git remote -v | grep "^jweyrich" # this is just for debugging
+'
+
test_expect_success 'remote set-url --push bar aaa' '
git remote set-url --push someremote bar aaa &&
echo foo >expect &&
--
1.8.1.421.g6236851
^ permalink raw reply related
* Re: [BUG] Possible bug in `remote set-url --add --push`
From: Jardel Weyrich @ 2013-01-12 8:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vliby98r7.fsf@alter.siamese.dyndns.org>
Step 1:
jweyrich@pharao:test_clone1 [* master]$ git remote -v
origin /Volumes/sandbox/test (fetch)
origin /Volumes/sandbox/test (push)
jweyrich@pharao:test_clone1 [* master]$ git config -l | grep '^remote\.origin'
remote.origin.url=/Volumes/sandbox/test
remote.origin.fetch=+refs/heads/*:refs/remotes/origin/*
Step 3:
jweyrich@pharao:test_clone1 [* master]$ git remote set-url --add
--push origin /Volumes/sandbox/test_clone2
origin /Volumes/sandbox/test (fetch)
origin /Volumes/sandbox/test_clone2/ (push)
jweyrich@pharao:test_clone1 [* master]$ git config -l | grep '^remote\.origin'
remote.origin.url=/Volumes/sandbox/test
remote.origin.fetch=+refs/heads/*:refs/remotes/origin/*
remote.origin.pushurl=/Volumes/sandbox/test_clone2/
After that, if I do a commit in test_clone1 and try to push to origin,
it pushes only to the test_clone2 rather than pushing to both test and
test_clone2 (it's a bare repo, sorry for using a misleading name).
Is `remote.<remote_name>.pushurl` required for the primary URL as
well? If not, then git-push is not handling that information as it
should.
On Sat, Jan 12, 2013 at 5:10 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Jardel Weyrich <jweyrich@gmail.com> writes:
>
>> I believe `remote set-url --add --push` has a bug. Performed tests
>> with v1.8.0.1 and v1.8.1 (Mac OS X).
>>
>> Quoting the relevant part of the documentation:
>>
>>> set-url
>>> Changes URL remote points to. Sets first URL remote points to matching regex <oldurl> (first URL if no <oldurl> is given) to <newurl>. If <oldurl> doesn’t match any URL, error occurs and nothing is changed.
>>>
>>> With --push, push URLs are manipulated instead of fetch URLs.
>>> With --add, instead of changing some URL, new URL is added.
>>> With --delete, instead of changing some URL, all URLs matching regex <url> are deleted. Trying to delete all non-push URLs is an error.
>>
>> Here are some steps to reproduce:
>>
>> 1. Show the remote URLs
>>
>> jweyrich@pharao:test_clone1 [* master]$ git remote -v
>> origin /Volumes/sandbox/test (fetch)
>> origin /Volumes/sandbox/test (push)
>>
>> 2. Add a new push URL for origin
>>
>> jweyrich@pharao:test_clone1 [* master]$ git remote set-url --add --push origin \
>> /Volumes/sandbox/test_clone2
>>
>> 3. Check what happened
>>
>> jweyrich@pharao:test_clone1 [* master]$ git remote -v
>> origin /Volumes/sandbox/test (fetch)
>> origin /Volumes/sandbox/test_clone2 (push)
>
> The original pushurl was replaced with the additional one, instead
> of being left and the new one getting added. That looks certainly
> wrong.
>
> However, the result of applying the attached patch (either to
> v1.7.12 or v1.8.1) still passes the test and I do not think it is
> doing anything differently from what you described above.
>
> What do you get from
>
> git config -l | grep '^remote\.origin'
>
> in steps 1. and 3. in your procedure? This question is trying to
> tell if your bug is in "git remote -v" or in "git remote set-url".
>
> -- >8 --
> From 0f6cbc67db926e97707ae732b02e790b4604508e Mon Sep 17 00:00:00 2001
> From: Junio C Hamano <gitster@pobox.com>
> Date: Fri, 11 Jan 2013 23:04:16 -0800
> Subject: [PATCH] t5505: adding one pushurl from jweyrich
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> t/t5505-remote.sh | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
> index c03ffdd..b31c5bb 100755
> --- a/t/t5505-remote.sh
> +++ b/t/t5505-remote.sh
> @@ -901,6 +901,25 @@ test_expect_success 'remote set-url --push --add aaa' '
> cmp expect actual
> '
>
> +test_expect_success 'remote set-url --push --add' '
> + git config remote.jweyrich.url /Volumes/sandbox/test &&
> + git config remote.jweyrich.pushurl /Volumes/sandbox/test &&
> + git config remote.jweyrich.fetch "refs/heads/*:refs/remotes/jweyrich/*" &&
> +
> + added=/Volumes/sandbox/test_clone2 &&
> + {
> + git config -l | grep "^remote\.jweyrich\." &&
> + echo "remote.jweyrich.pushurl=$added"
> + } | sort >expect &&
> +
> + git remote set-url --add --push jweyrich "$added" &&
> + git config -l | grep "^remote\.jweyrich\." | sort >actual &&
> +
> + test_cmp expect actual &&
> +
> + git remote -v | grep "^jweyrich" # this is just for debugging
> +'
> +
> test_expect_success 'remote set-url --push bar aaa' '
> git remote set-url --push someremote bar aaa &&
> echo foo >expect &&
> --
> 1.8.1.421.g6236851
^ permalink raw reply
* Re: [BUG] Possible bug in `remote set-url --add --push`
From: Junio C Hamano @ 2013-01-12 8:23 UTC (permalink / raw)
To: Jardel Weyrich; +Cc: git
In-Reply-To: <CAN8TAOvP_HX6BEK86aYoX-kVqWDmsbyptxTT2nk+fx+Ut1Tojg@mail.gmail.com>
Jardel Weyrich <jweyrich@gmail.com> writes:
> Step 1:
>
> jweyrich@pharao:test_clone1 [* master]$ git remote -v
> origin /Volumes/sandbox/test (fetch)
> origin /Volumes/sandbox/test (push)
>
> jweyrich@pharao:test_clone1 [* master]$ git config -l | grep '^remote\.origin'
> remote.origin.url=/Volumes/sandbox/test
> remote.origin.fetch=+refs/heads/*:refs/remotes/origin/*
>
> Step 3:
>
> jweyrich@pharao:test_clone1 [* master]$ git remote set-url --add
> --push origin /Volumes/sandbox/test_clone2
> origin /Volumes/sandbox/test (fetch)
> origin /Volumes/sandbox/test_clone2/ (push)
>
> jweyrich@pharao:test_clone1 [* master]$ git config -l | grep '^remote\.origin'
> remote.origin.url=/Volumes/sandbox/test
> remote.origin.fetch=+refs/heads/*:refs/remotes/origin/*
> remote.origin.pushurl=/Volumes/sandbox/test_clone2/
So "remote -v" is not lying (we only see one pushurl after Step 3
above) and "set-url" is not working correctly on your box in a way
that I cannot reproduce X-<.
> ...
> Is `remote.<remote_name>.pushurl` required for the primary URL as
> well?
What do you mean by "primary URL"?
^ permalink raw reply
* [PATCH] t/lib-cvs: cvsimport no longer works without Python >= 2.7
From: Junio C Hamano @ 2013-01-12 8:40 UTC (permalink / raw)
To: Eric S. Raymond; +Cc: git
In-Reply-To: <7v62339du4.fsf@alter.siamese.dyndns.org>
The new cvsimport requires at least Python 2.7 to work; do not fail
the cvsimport tests on platforms without one.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Junio C Hamano <gitster@pobox.com> writes:
> http://docs.python.org/2/library/subprocess.html tells me that
> check_output has first become available in 2.7.
>
> So... does this mean that we now set the minimum required version of
> Python to 2.7? I dunno.
Even if we were to rip out the fallback code that uses the 2.7-only
subprocess.check_output() on "cvsps -V", the function is also used
for doing the real work interacting with cvsps-3.x, so I think this
patch will be necessary. Unless new cvsimport is tweaked not to
use the method, that is.
A suggestion for a better alternative is of course very much
appreciated.
I do not want to keep pushing integration results that do not pass
tests even for myself for too many integration cycles, so I'll keep
this patch at the tip of the topic, at least for now.
t/lib-cvs.sh | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/t/lib-cvs.sh b/t/lib-cvs.sh
index b55e861..4e890ea 100644
--- a/t/lib-cvs.sh
+++ b/t/lib-cvs.sh
@@ -27,6 +27,21 @@ case "$cvsps_version" in
;;
esac
+if ! test_have_prereq PYTHON
+then
+ skipall='skipping cvsimport tests, no python'
+ test_done
+fi
+
+python -c '
+import sys
+if sys.hexversion < 0x02070000:
+ sys.exit(1)
+' || {
+ skip_all='skipping cvsimport tests, python too old (< 2.7)'
+ test_done
+}
+
setup_cvs_test_repository () {
CVSROOT="$(pwd)/.cvsroot" &&
cp -r "$TEST_DIRECTORY/$1/cvsroot" "$CVSROOT" &&
--
1.8.1.421.g6236851
^ permalink raw reply related
* Re: [BUG] Possible bug in `remote set-url --add --push`
From: Sascha Cunz @ 2013-01-12 8:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jardel Weyrich, git
In-Reply-To: <7vliby98r7.fsf@alter.siamese.dyndns.org>
Am Freitag, 11. Januar 2013, 23:10:36 schrieb Junio C Hamano:
> Jardel Weyrich <jweyrich@gmail.com> writes:
> > I believe `remote set-url --add --push` has a bug. Performed tests
> > with v1.8.0.1 and v1.8.1 (Mac OS X).
> >
> > Quoting the relevant part of the documentation:
> >> set-url
> >>
> >> Changes URL remote points to. Sets first URL remote points to
> >> matching regex <oldurl> (first URL if no <oldurl> is given) to
> >> <newurl>. If <oldurl> doesn’t match any URL, error occurs and
> >> nothing is changed.
> >>
> >> With --push, push URLs are manipulated instead of fetch URLs.
> >> With --add, instead of changing some URL, new URL is added.
> >> With --delete, instead of changing some URL, all URLs matching regex
> >> <url> are deleted. Trying to delete all non-push URLs is an error.>
> > Here are some steps to reproduce:
> >
> > 1. Show the remote URLs
> >
> > jweyrich@pharao:test_clone1 [* master]$ git remote -v
> > origin /Volumes/sandbox/test (fetch)
> > origin /Volumes/sandbox/test (push)
> >
> > 2. Add a new push URL for origin
> >
> > jweyrich@pharao:test_clone1 [* master]$ git remote set-url --add --push
> > origin \>
> > /Volumes/sandbox/test_clone2
> >
> > 3. Check what happened
> >
> > jweyrich@pharao:test_clone1 [* master]$ git remote -v
> > origin /Volumes/sandbox/test (fetch)
> > origin /Volumes/sandbox/test_clone2 (push)
>
> The original pushurl was replaced with the additional one, instead
> of being left and the new one getting added. That looks certainly
> wrong.
>
> However, the result of applying the attached patch (either to
> v1.7.12 or v1.8.1) still passes the test and I do not think it is
> doing anything differently from what you described above.
>
> What do you get from
>
> git config -l | grep '^remote\.origin'
>
> in steps 1. and 3. in your procedure? This question is trying to
> tell if your bug is in "git remote -v" or in "git remote set-url".
I'm not sure, if there is a bug at all. According to man git-push:
The <pushurl> is used for pushes only. It is optional and defaults to
<url>.
(From the section REMOTES -> Named remote in configuration file)
the command:
git remote add foo git@foo-fetch.org/some.git
will set "remote.foo.url" to "git@foo-fetch.org". Subsequently, fetch and push
will use git@foo-fetch.org as url.
Fetch will use this url, because "remote.foo.url" explicitly sets this. push
will use it in absence of a "remote.foo.pushurl".
Now, we're adding a push-url:
git remote set-url --add --push foo git@foo-push.org/some.git
Relevant parts of config are now looking like:
[remote "foo"]
url = git@foo-fetch.org/some.git
pushurl = git@foo-push.org/some.git
Since, pushurl is now given explicitly, git push will use that one (and only
that one).
If we add another push-url now,
git remote set-url --add --push foo git@foo-push-also.org/some.git
the next git-push will push to foo-push.org and foo-push-also.org.
Now, using --set-url --delete on both of these urls restores the original
state: only "remote.foo.url" is set; meaning implicitly pushurl defaults to
url again.
To me this is exactly what Jardel was observing:
> In step 2, Git replaced the original push URL instead of adding a new
> one. But it seems to happen only the first time I use `remote set-url
> --add --push`. Re-adding the original URL using the same command seems
> to work properly.
> And FWIW, if I delete (with "set-url --delete") both URLs push, Git
> restores the original URL.
Or am I missing something here?
Might be that the "bug" actually is that the expectation was
git remote add foo git@foo-fetch.org/some.git
should have created a config like:
[remote "foo"]
url = git@foo-fetch.org/some.git
pushurl = git@foo-fetch.org/some.git
since that is what "git remote -v" reports.
If that is the case, we might want to amend the output of 'git remote -v' with
the information that a pushurl is not explicitly given and thus defaults to
url.
Sascha
^ permalink raw reply
* Re: [BUG] Possible bug in `remote set-url --add --push`
From: Jardel Weyrich @ 2013-01-12 9:33 UTC (permalink / raw)
To: Sascha Cunz; +Cc: Junio C Hamano, git
In-Reply-To: <4836187.09xoy3kJnj@blacky>
On Sat, Jan 12, 2013 at 6:44 AM, Sascha Cunz <sascha-ml@babbelbox.org> wrote:
> Am Freitag, 11. Januar 2013, 23:10:36 schrieb Junio C Hamano:
>> Jardel Weyrich <jweyrich@gmail.com> writes:
>> > I believe `remote set-url --add --push` has a bug. Performed tests
>> > with v1.8.0.1 and v1.8.1 (Mac OS X).
>> >
>> > Quoting the relevant part of the documentation:
>> >> set-url
>> >>
>> >> Changes URL remote points to. Sets first URL remote points to
>> >> matching regex <oldurl> (first URL if no <oldurl> is given) to
>> >> <newurl>. If <oldurl> doesn’t match any URL, error occurs and
>> >> nothing is changed.
>> >>
>> >> With --push, push URLs are manipulated instead of fetch URLs.
>> >> With --add, instead of changing some URL, new URL is added.
>> >> With --delete, instead of changing some URL, all URLs matching regex
>> >> <url> are deleted. Trying to delete all non-push URLs is an error.>
>> > Here are some steps to reproduce:
>> >
>> > 1. Show the remote URLs
>> >
>> > jweyrich@pharao:test_clone1 [* master]$ git remote -v
>> > origin /Volumes/sandbox/test (fetch)
>> > origin /Volumes/sandbox/test (push)
>> >
>> > 2. Add a new push URL for origin
>> >
>> > jweyrich@pharao:test_clone1 [* master]$ git remote set-url --add --push
>> > origin \>
>> > /Volumes/sandbox/test_clone2
>> >
>> > 3. Check what happened
>> >
>> > jweyrich@pharao:test_clone1 [* master]$ git remote -v
>> > origin /Volumes/sandbox/test (fetch)
>> > origin /Volumes/sandbox/test_clone2 (push)
>>
>> The original pushurl was replaced with the additional one, instead
>> of being left and the new one getting added. That looks certainly
>> wrong.
>>
>> However, the result of applying the attached patch (either to
>> v1.7.12 or v1.8.1) still passes the test and I do not think it is
>> doing anything differently from what you described above.
>>
>> What do you get from
>>
>> git config -l | grep '^remote\.origin'
>>
>> in steps 1. and 3. in your procedure? This question is trying to
>> tell if your bug is in "git remote -v" or in "git remote set-url".
>
> I'm not sure, if there is a bug at all. According to man git-push:
>
> The <pushurl> is used for pushes only. It is optional and defaults to
> <url>.
> (From the section REMOTES -> Named remote in configuration file)
>
> the command:
> git remote add foo git@foo-fetch.org/some.git
>
> will set "remote.foo.url" to "git@foo-fetch.org". Subsequently, fetch and push
> will use git@foo-fetch.org as url.
> Fetch will use this url, because "remote.foo.url" explicitly sets this. push
> will use it in absence of a "remote.foo.pushurl".
>
> Now, we're adding a push-url:
> git remote set-url --add --push foo git@foo-push.org/some.git
>
> Relevant parts of config are now looking like:
> [remote "foo"]
> url = git@foo-fetch.org/some.git
> pushurl = git@foo-push.org/some.git
>
> Since, pushurl is now given explicitly, git push will use that one (and only
> that one).
>
> If we add another push-url now,
> git remote set-url --add --push foo git@foo-push-also.org/some.git
>
> the next git-push will push to foo-push.org and foo-push-also.org.
>
> Now, using --set-url --delete on both of these urls restores the original
> state: only "remote.foo.url" is set; meaning implicitly pushurl defaults to
> url again.
>
> To me this is exactly what Jardel was observing:
>
>> In step 2, Git replaced the original push URL instead of adding a new
>> one. But it seems to happen only the first time I use `remote set-url
>> --add --push`. Re-adding the original URL using the same command seems
>> to work properly.
>
>> And FWIW, if I delete (with "set-url --delete") both URLs push, Git
>> restores the original URL.
>
> Or am I missing something here?
You're right. However, as I quoted earlier, the git-remote man-page states:
set-url
Changes URL remote points to. <suppressed>
With --push, push URLs are manipulated instead of fetch URLs.
With --add, instead of changing some URL, new URL is added.
It explicitly mentions that it should **add a new URL**.
So when I do `git remote set-url --add --push origin
git://another/repo.git`, I expect git-push to use both the default
push URL and the new one. Or am I misinterpreting the man-page?
>
> Might be that the "bug" actually is that the expectation was
>
> git remote add foo git@foo-fetch.org/some.git
>
> should have created a config like:
>
> [remote "foo"]
> url = git@foo-fetch.org/some.git
> pushurl = git@foo-fetch.org/some.git
>
> since that is what "git remote -v" reports.
>
> If that is the case, we might want to amend the output of 'git remote -v' with
> the information that a pushurl is not explicitly given and thus defaults to
> url.
Correct. Adding a remote doesn't automatically generate a pushurl for it.
To me, it seems that git-push checks for the existence of pushurl's in
the config, and if it finds any, it ignores the defaul push URL during
the actual push. In better words, it pushes only to pushurls, if it
finds any, otherwise it pushes to the default URL.
To comply with the statements in the git-remote man-page, git-remote
should add a pushurl configuration containing the default push URL,
along with the one passed to `set-url --add --push`. Or git-push
should _not ignore_ the default URL in the presence of pushurls,
effectively pushing to both. These are the solutions I can think of
right now, supposing I'm correct about the cause(s).
>
> Sascha
^ permalink raw reply
* Re: [PATCH v5] git-completion.bash: add support for path completion
From: Manlio Perillo @ 2013-01-12 12:53 UTC (permalink / raw)
To: Manlio Perillo; +Cc: git, gitster, szeder, felipe.contreras, peff
In-Reply-To: <1357930123-26310-1-git-send-email-manlio.perillo@gmail.com>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Il 11/01/2013 19:48, Manlio Perillo ha scritto:
> 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.
> [...]
>
> +# 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
> + --)
Sorry, I have incorrectly (again) indented the case labels.
I have now configured my editor to correctly indent this.
> + # Good; we can assume that the following are only non
> + # option arguments.
> + ((c = 0))
> + ;;
Here I was thinking to do something like this (not tested):
-*)
if [ -n ${2-} ]; then
# Assume specified git command only
# accepts simple options
# (without arguments)
((c = 0))
Since git mv only accepts simple options, this will make the use of '--'
not required.
Note that I'm assuming the single '-' character is used as a non-option
argument; not sure this is the case of Git.
> [...]
Regards Manlio
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAlDxXLkACgkQscQJ24LbaUR+QQCaA4WZP5h5lktXJqSB7c494fAY
B6IAoIRWyIzBq29S7+l+TfRjbyp19HNL
=JRpR
-----END PGP SIGNATURE-----
^ permalink raw reply
* Re: missing objects -- prevention
From: Jeff King @ 2013-01-12 13:13 UTC (permalink / raw)
To: Sitaram Chamarty; +Cc: Git Mailing List
In-Reply-To: <CAMK1S_jN7=Antz-5D7yf0KV8m-YEy93tZP_zziTXPDzbdyjUrw@mail.gmail.com>
On Sat, Jan 12, 2013 at 06:39:52AM +0530, Sitaram Chamarty wrote:
> > 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.
>
> The gitolite logs show that no deletion of refs has happened.
To be pedantic, step 3 could also be rewinding R to a commit before X.
Anything that causes X to become unreferenced.
> > 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.
>
> This is for the packed-refs file right? And it could result in a ref
> getting deleted right?
Yes, if the ref was not previously packed, it could result in the ref
being deleted entirely.
> I said above that the gitolite logs say no ref was deleted. What if
> the ref "deletion" happened because of this race, making the rest of
> your 4-step scenario above possible?
It's possible. I do want to highlight how unlikely it is, though.
> > up in the middle, or fsck rejects the pack). We have historically left
>
> fsck... you mean if I had 'receive.fsckObjects' true, right? I don't.
> Should I? Would it help this overall situation? As I understand it,
> thats only about the internals of each object to check corruption, and
> cannot detect a *missing* object on the local object store.
Right, I meant if you have receive.fsckObjects on. It won't help this
situation at all, as we already do a connectivity check separate from
the fsck. But I do recommend it in general, just because it helps catch
bad objects before they gets disseminated to a wider audience (at which
point it is often infeasible to rewind history). And it has found git
bugs (e.g., null sha1s in tree entries).
> > 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
>
> OK; I'll do the same then. I suppose a cron job is the best way; I
> didn't find any config for expiring these files.
If you run "git prune --expire=1.hour.ago", it should prune stale
tmp_pack_* files more than an hour old. But you may not be comfortable
with such a short expiration for the objects themselves. :)
> Thanks again for your help. I'm going to treat it (for now) as a
> disk/fs error after hearing from you about the other possibility I
> mentioned above, although I find it hard to believe one repo can be
> hit buy *two* races occurring together!
Yeah, the race seems pretty unlikely (though it could be just the one
race with a rewind). As I said, I haven't actually ever seen it in
practice. In my experience, though, disk/fs issues do not manifest as
just missing objects, but as corrupted packfiles (e.g., the packfile
directory entry ends up pointing to the wrong inode, which is easy to
see because the inode's content is actually a reflog). And then of
course with the packfile unreadable, you have missing objects. But YMMV,
depending on the fs and what's happened to the machine to cause the fs
problem.
-Peff
^ permalink raw reply
* [PATCH] Support FTP-over-SSL/TLS for regular FTP
From: Modestas Vainius @ 2013-01-12 13:59 UTC (permalink / raw)
To: git; +Cc: Modestas Vainius
Add a boolean http.sslTry option which allows to enable AUTH SSL/TLS and
encrypted data transfers when connecting via regular FTP protocol.
Default is false since it might trigger certificate verification errors on
misconfigured servers.
Signed-off-by: Modestas Vainius <modestas@vainius.eu>
---
Documentation/config.txt | 8 ++++++++
http.c | 10 ++++++++++
http.h | 9 +++++++++
3 files changed, 27 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index d5809e0..1abd161 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1406,6 +1406,14 @@ http.sslCAPath::
with when fetching or pushing over HTTPS. Can be overridden
by the 'GIT_SSL_CAPATH' environment variable.
+http.sslTry::
+ Attempt to use AUTH SSL/TLS and encrypted data transfers
+ when connecting via regular FTP protocol. This might be needed
+ if the FTP server requires it for security reasons or you wish
+ to connect securely whenever remote FTP server supports it.
+ Default is false since it might trigger certificate verification
+ errors on misconfigured servers.
+
http.maxRequests::
How many HTTP requests to launch in parallel. Can be overridden
by the 'GIT_HTTP_MAX_REQUESTS' environment variable. Default is 5.
diff --git a/http.c b/http.c
index 44f3525..d49a3d4 100644
--- a/http.c
+++ b/http.c
@@ -30,6 +30,7 @@ static CURL *curl_default;
char curl_errorstr[CURL_ERROR_SIZE];
static int curl_ssl_verify = -1;
+static int curl_ssl_try;
static const char *ssl_cert;
#if LIBCURL_VERSION_NUM >= 0x070903
static const char *ssl_key;
@@ -162,6 +163,10 @@ static int http_options(const char *var, const char *value, void *cb)
ssl_cert_password_required = 1;
return 0;
}
+ if (!strcmp("http.ssltry", var)) {
+ curl_ssl_try = git_config_bool(var, value);
+ return 0;
+ }
if (!strcmp("http.minsessions", var)) {
min_curl_sessions = git_config_int(var, value);
#ifndef USE_CURL_MULTI
@@ -306,6 +311,11 @@ static CURL *get_curl_handle(void)
if (curl_ftp_no_epsv)
curl_easy_setopt(result, CURLOPT_FTP_USE_EPSV, 0);
+#ifdef CURLOPT_USE_SSL
+ if (curl_ssl_try)
+ curl_easy_setopt(result, CURLOPT_USE_SSL, CURLUSESSL_TRY);
+#endif
+
if (curl_http_proxy) {
curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);
diff --git a/http.h b/http.h
index 0a80d30..f861662 100644
--- a/http.h
+++ b/http.h
@@ -42,6 +42,15 @@
#define NO_CURL_IOCTL
#endif
+/*
+ * CURLOPT_USE_SSL was known as CURLOPT_FTP_SSL up to 7.16.4,
+ * and the constants were known as CURLFTPSSL_*
+*/
+#if !defined(CURLOPT_USE_SSL) && defined(CURLOPT_FTP_SSL)
+#define CURLOPT_USE_SSL CURLOPT_FTP_SSL
+#define CURLUSESSL_TRY CURLFTPSSL_TRY
+#endif
+
struct slot_results {
CURLcode curl_result;
long http_code;
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH] Support FTP-over-SSL/TLS for regular FTP
From: Matt Kraai @ 2013-01-12 14:25 UTC (permalink / raw)
To: Modestas Vainius; +Cc: git
In-Reply-To: <1357999192-877-1-git-send-email-modestas@vainius.eu>
On Sat, Jan 12, 2013 at 03:59:52PM +0200, Modestas Vainius wrote:
> @@ -306,6 +311,11 @@ static CURL *get_curl_handle(void)
> if (curl_ftp_no_epsv)
> curl_easy_setopt(result, CURLOPT_FTP_USE_EPSV, 0);
>
> +#ifdef CURLOPT_USE_SSL
> + if (curl_ssl_try)
> + curl_easy_setopt(result, CURLOPT_USE_SSL, CURLUSESSL_TRY);
> +#endif
> +
> if (curl_http_proxy) {
> curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
> curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);
It looks like the indentation of the "if" line you added is messed up.
^ permalink raw reply
* How to setup bash completion for alias of git command
From: Ping Yin @ 2013-01-12 14:30 UTC (permalink / raw)
To: git mailing list
Following setup works for me in ubuntu (10.04,11.04) for a long time
alias gtlg='git log'
complete -o default -o nospace -F _git_log gtlg
However, in debian (testing, wheezy), it doesn't work
$ gtlg or<TAB>
gtlg or-bash: [: 1: unary operator expected
-bash: [: 1: unary operator expected
$ git --version
git version 1.7.10
Can anybody help?
^ permalink raw reply
* Re: [PATCH] Support FTP-over-SSL/TLS for regular FTP
From: Modestas Vainius @ 2013-01-12 14:51 UTC (permalink / raw)
To: Matt Kraai; +Cc: git
In-Reply-To: <20130112142521.GA21639@ftbfs.org>
Hello,
Saturday 12 January 2013 06:25:21 rašė:
> On Sat, Jan 12, 2013 at 03:59:52PM +0200, Modestas Vainius wrote:
> > @@ -306,6 +311,11 @@ static CURL *get_curl_handle(void)
> >
> > if (curl_ftp_no_epsv)
> >
> > curl_easy_setopt(result, CURLOPT_FTP_USE_EPSV, 0);
> >
> > +#ifdef CURLOPT_USE_SSL
> > + if (curl_ssl_try)
> > + curl_easy_setopt(result, CURLOPT_USE_SSL, CURLUSESSL_TRY);
> > +#endif
> > +
> >
> > if (curl_http_proxy) {
> >
> > curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
> > curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);
>
> It looks like the indentation of the "if" line you added is messed up.
Yeah, sorry about that. I will fix it.
--
Modestas Vainius <modestas@vainius.eu>
^ permalink raw reply
* Re: [PATCH v5] git-completion.bash: add support for path completion
From: Manlio Perillo @ 2013-01-12 14:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, szeder, felipe.contreras, peff
In-Reply-To: <7v8v7zbcoi.fsf@alter.siamese.dyndns.org>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Il 11/01/2013 23:02, Junio C Hamano ha scritto:
> Manlio Perillo <manlio.perillo@gmail.com> writes:
>
>> +# 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
>
> I think this redirection is correct but a bit tricky;
It's not tricky: it is POSIX:
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_10
> it is in
> effect during the execution of the { block } (in other words, it is
> not about squelching errors during the function definition).
>
What do you mean by "squelching"?
Note that I originally wrote the code as
__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
}
but then I checked the POSIX standard, noting that it is redundant.
> -- >8 --
> #!/bin/sh
> cat >t.sh <<\EOF &&
> echo I am "$1"
> t () { echo "Goes to stdout"; echo >&2 "Goes to stderr"; } 2>/dev/null
> t
> for sh in bash dash ksh zsh
> do
> $sh t.sh $sh
> done
> -- 8< --
>
There is a missing EOF delimiter.
And I'm not sure to understand the meaning of && after EOF.
> Bash does (so do dash and real AT&T ksh) grok this correctly, but
> zsh does not seem to (I tried zsh 4.3.10 and 4.3.17; also zsh
> pretending to be ksh gets this wrong as well). Not that what ksh
> does matters, as it won't be dot-sourcing bash completion script.
>
I have added tcsh to the sh list, but it fails with:
Badly placed ()'s.
> It however may affect zsh, which does seem to dot-source this file.
> Perhaps zsh completion may have to be rewritten in a similar way as
> tcsh completion is done (i.e. does not dot-source this file but ask
> bash to do the heavy-lifting).
>
Ok, I was wrong on assuming all modern shells were POSIX compliant.
I will change the code to use a nested {} group.
> This function seems to be always called in an subshell (e.g. as an
> upstream of a pipeline), so the "cd" may be harmless, but don't you
> need to disable CDPATH while doing this?
>
I don't know.
> [..]
>> +# 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
>
> If that is the case, it is a bug in the command line parser, I
> think. We should reject it, and the command line completer
> certainly should not encourage it.
>
$ mkdir y
$ git mv x -n y
Checking rename of 'x' to 'y/x'
Renaming x to y/x
$ git status
# On branch master
nothing to commit, working directory clean
I was assuming it to be "normal", given how complex Git command line
parsing is (IMHO).
Thanks Manlio
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAlDxeMgACgkQscQJ24LbaUTmaQCeMbZ0lRJxZIx3U31gMPmcqTLp
54sAmwYrjJVuvRYcsbGaMa3rb9/EKrBU
=ky30
-----END PGP SIGNATURE-----
^ permalink raw reply
* Re: [PATCH] cvsimport: rewrite to use cvsps 3.x to fix major bugs
From: Michael Haggerty @ 2013-01-12 15:13 UTC (permalink / raw)
To: Eric S. Raymond; +Cc: Junio C Hamano, git, Chris Rorvick
In-Reply-To: <1357875152-19899-1-git-send-email-gitster@pobox.com>
On 01/11/2013 04:32 AM, Junio C Hamano wrote:
> From: "Eric S. Raymond" <esr@thyrsus.com>
>
> The combination of git-cvsimport and cvsps had serious problems.
Agreed.
> [...]
> This patch also removes Michael Haggerty's git-cvsimport tests
> (t960[123]) from the git tree. These are actually conversion-engine
> tests and have been merged into a larger cvsps test suite, which I
> intend to spin out into a general CVS-lifting test that can also be
> applied to utilities such as cvs2git and parsecvs. The t9604 test
> will move in a future patch, when I likewise have it integrated
> into the general test suite.
>
> The following known bug has not been fixed: "If any files were ever
> "cvs import"ed more than once (e.g., import of more than one vendor
> release) the HEAD contains the wrong content." However, cvsps now
> emits a warning in this case. There is also one pathological tagging
> case that was successful in the former t9602 test that now fails
> (with a warning).
>
> I plan to address these problems. This patch at least gets the
> cvsps-3.x/git-cvsimport combination to a state that is not too
> broken to ship - that is, in all failure cases known to me it
> now emits useful warnings rather than silently botching the
> import.
I don't understand the logic of removing the cvsimport tests, at least
not at this time. It is true that the tests mostly ensure that the
conversion engine is working correctly, especially with your new version
of cvsps. But I think the git project, by implicitly endorsing the use
of cvsps, has some responsibility to verify that the combination cvsps +
git-cvsimport continues to work and to document any known breakages via
its test suite.
Otherwise, how do we know that cvsps currently works with git-cvsimport?
(OK, you claim that it does, but in the next breath you admit that
there is a new failure in "one pathological tagging case".) How can we
understand its strengths/weaknesses? How can we gain confidence that it
works on different platforms? How will we find out if a future versions
of cvsps stops working (e.g., because of a breakage or a
non-backwards-compatible change)?
Normally one would expect an improvement like this to be combined with
patches that turn test expected failures into expected successes, not to
rip out the very tests that establish the correctness of the change that
is being proposed!
Let me describe what I would consider to be the optimum state of the
test suite. Maybe your idea of "optimum" differs from mine, or maybe
the optimum is unrealistic due to lack of resources or for some other
reason. But if so, let's explicitly spell out why we are deviating from
whatever optimum we define.
* The old tests should be retained (and possibly new tests added to show
off your improvements).
* There should be a way for users to choose which cvsps executable to
use when running test suite. (In the future, the selection might be
expanded to cover altogether different conversion engines.)
* The tests should determine which version of cvsps has been selected
(e.g., by running "cvsps --version").
* The individual tests should be marked expected success/expected
failure based on the selected version of cvsps; in other words, some
tests might be marked "expected failure" if cvsps 2.x is being used but
"expected success" if cvsps 3.x is being used.
Regarding your claim that "within a few months the Perl git-cvsimport is
going to cease even pretending to work": It might be that the old
git-cvsimport will stop working *for people who upgrade to cvsps 3.x*.
But it is not realistic to expect people to synchronize their git and
cvsps version upgrades. It is even quite possible that this or that
Linux distribution will package incompatible versions of the two packages.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: [PATCH] t/lib-cvs: cvsimport no longer works without Python >= 2.7
From: Michael Haggerty @ 2013-01-12 15:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Eric S. Raymond, git
In-Reply-To: <7vip72iykx.fsf_-_@alter.siamese.dyndns.org>
I have the feeling I'm only seeing one side of this conversation...
On 01/12/2013 09:40 AM, Junio C Hamano wrote:
> The new cvsimport requires at least Python 2.7 to work; do not fail
> the cvsimport tests on platforms without one.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>
> Junio C Hamano <gitster@pobox.com> writes:
>
> > http://docs.python.org/2/library/subprocess.html tells me that
> > check_output has first become available in 2.7.
> >
> > So... does this mean that we now set the minimum required version of
> > Python to 2.7? I dunno.
It would be unfortunate to set the minimum Python version to 2.7 if "git
cvsimport" is considered an integral part of git.
> Even if we were to rip out the fallback code that uses the 2.7-only
> subprocess.check_output() on "cvsps -V", the function is also used
> for doing the real work interacting with cvsps-3.x, so I think this
> patch will be necessary. Unless new cvsimport is tweaked not to
> use the method, that is.
>
> A suggestion for a better alternative is of course very much
> appreciated.
If the only reason to require Python 2.7 is subprocess.check_output(),
it would be easy to reimplement it (it is only 12 lines of
straightforward code, plus a few lines to define the exception type
CalledProcessError). According to [1], the Python license is
GPL-compatible; therefore these lines could even be copied into
git-cvsimport.
Michael
[1] http://www.gnu.org/licenses/license-list.html#Python
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: [PATCH] cvsimport: rewrite to use cvsps 3.x to fix major bugs
From: Eric S. Raymond @ 2013-01-12 15:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v62339du4.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com>:
> And here is what I got:
Hm. In my version of these tests, I only have one regression from the
old combo (in the pathological tags test, t9602). You're seeing more
breakage than that, obviously.
> A funny thing was that without cvsps-3.7 on $PATH (which means I am
> getting distro packaged cvsps 2.1), I got identical errors.
That suggests that something in your test setup has gone bad and is
introducing spurious errors. Which would be consistent with the above.
> Looking
> at the log message, it seems that you meant to remove t960[123], so
> perhaps the patch simply forgot to remove 9601 and 9602?
Yes.
> As neither test runs "git cvsimport" with -o/-m/-M options, ideally
> we should be able to pass them with and without having cvsps-3.x.
> Not passing them without cvsps-3.x would mean that the fallback mode
> of rewritten cvsimport is not working as expected. Not passing them
> with cvsps-3.x may mean the tests were expecting a wrong conversion
> result, or they uncover bugs in the replacement cvsimport.
That's possible, but seems unlikely. Because the new cvsimport is
such a thin wrapper around the conversion engine, bugs in it should
lead to obvious crashes or failure to run the engine rather than the
sort of conversion error the t960* tests are designed to check. Really
all it does is assemble options to pass to the conversion engines.
My test strategy is aimed at the engine, not the wrapper. I took the
repos from t960* and wrote a small Python framework to check the same
assertions as the git-tree tests do, but using the engine. For example,
here's how my t9602 looks:
import os, cvspstest
cc = cvspstest.ConvertComparison("t9602", "module")
cc.cmp_branch_tree("test of branch", "master", True)
cc.cmp_branch_tree("test of branch", "vendorbranch", True)
cc.cmp_branch_tree("test of branch", "B_FROM_INITIALS", False)
cc.cmp_branch_tree("test of branch", "B_FROM_INITIALS_BUT_ONE", False)
cc.cmp_branch_tree("test of branch", "B_MIXED", False)
cc.cmp_branch_tree("test of branch", "B_SPLIT", True)
cc.cmp_branch_tree("test of tag", "vendortag", False)
# This is the only test new cvsps fails that old git-cvsimport passed.
cc.cmp_branch_tree("test of tag", "T_ALL_INITIAL_FILES", True)
cc.cmp_branch_tree("test of tag", "T_ALL_INITIAL_FILES_BUT_ONE", False)
cc.cmp_branch_tree("test of tag", "T_MIXED", False)
cc.cleanup()
> t9600 fails with "-a is no longer supported", even without having
> cvsps-3.x on the $PATH (i.e. attempting to use the fallback). I
> wonder if this is an option the updated cvsimport would want to
> simply ignore?
Probably. But I don't think you should keep these tests in the git tree.
That wasn't a great idea even when you were supporting just one engine;
with two (and soon three) it's going to be just silly. Let sanity-checking
the engines be *my* problem, since I have to do it anyway.
(I'm working towards the generalized test suite as fast as I can. First
results probably in four days or so.)
> It is a way to tell the old cvsps/cvsimport to disable its
> heuristics to ignore commits made within the last 10 minutes (this
> is done in the hope of waiting for the per-file nature of CVS
> commits to stabilize, IIUC); the user tells the command that he
> knows that the CVS repository is now quiescent and it is safe to
> import the whole thing.
Yes, that's just what -a is supposed to do. But is should be
irrelevant for testing - in the test framework CVS is running locally,
so there's no network lag.
> So... does this mean that we now set the minimum required version of
> Python to 2.7? I dunno.
That would be bad, IMO. I'll put backporting to 2.6 high on my to-do list.
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
^ permalink raw reply
* [PATCH 2/3] config: Introduce diff.algorithm variable
From: Michal Privoznik @ 2013-01-12 16:02 UTC (permalink / raw)
To: git; +Cc: peff, trast
In-Reply-To: <cover.1358006339.git.mprivozn@redhat.com>
Some users or projects prefer different algorithms over others, e.g.
patience over myers or similar. However, specifying appropriate
argument every time diff is to be used is impractical. Moreover,
creating an alias doesn't play nicely with other tools based on diff
(git-show for instance). Hence, a configuration variable which is able
to set specific algorithm is needed. For now, these four values are
accepted: 'myers' (which has the same effect as not setting the config
variable at all), 'minimal', 'patience' and 'histogram'.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
Documentation/diff-config.txt | 17 +++++++++++++++++
contrib/completion/git-completion.bash | 1 +
diff.c | 23 +++++++++++++++++++++++
3 files changed, 41 insertions(+)
diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
index 4314ad0..be31276 100644
--- a/Documentation/diff-config.txt
+++ b/Documentation/diff-config.txt
@@ -155,3 +155,20 @@ diff.tool::
"kompare". Any other value is treated as a custom diff tool,
and there must be a corresponding `difftool.<tool>.cmd`
option.
+
+diff.algorithm::
+ Choose a diff algorithm. The variants are as follows:
++
+--
+`myers`;;
+ The basic greedy diff algorithm.
+`minimal`;;
+ Spend extra time to make sure the smallest possible diff is
+ produced.
+`patience`;;
+ Use "patience diff" algorithm when generating patches.
+`histogram`;;
+ This algorithm extends the patience algorithm to "support
+ low-occurrence common elements".
+--
++
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 383518c..33e25dc 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1839,6 +1839,7 @@ _git_config ()
diff.suppressBlankEmpty
diff.tool
diff.wordRegex
+ diff.algorithm
difftool.
difftool.prompt
fetch.recurseSubmodules
diff --git a/diff.c b/diff.c
index 732d4c2..ddae5c4 100644
--- a/diff.c
+++ b/diff.c
@@ -36,6 +36,7 @@ static int diff_no_prefix;
static int diff_stat_graph_width;
static int diff_dirstat_permille_default = 30;
static struct diff_options default_diff_options;
+static long diff_algorithm = 0;
static char diff_colors[][COLOR_MAXLEN] = {
GIT_COLOR_RESET,
@@ -143,6 +144,20 @@ static int git_config_rename(const char *var, const char *value)
return git_config_bool(var,value) ? DIFF_DETECT_RENAME : 0;
}
+static long parse_algorithm_value(const char *value)
+{
+ if (!value || !strcasecmp(value, "myers"))
+ return 0;
+ else if (!strcasecmp(value, "minimal"))
+ return XDF_NEED_MINIMAL;
+ else if (!strcasecmp(value, "patience"))
+ return XDF_PATIENCE_DIFF;
+ else if (!strcasecmp(value, "histogram"))
+ return XDF_HISTOGRAM_DIFF;
+ else
+ return -1;
+}
+
/*
* These are to give UI layer defaults.
* The core-level commands such as git-diff-files should
@@ -196,6 +211,13 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
return 0;
}
+ if (!strcmp(var, "diff.algorithm")) {
+ diff_algorithm = parse_algorithm_value(value);
+ if (diff_algorithm < 0)
+ return -1;
+ return 0;
+ }
+
if (git_color_config(var, value, cb) < 0)
return -1;
@@ -3213,6 +3235,7 @@ void diff_setup(struct diff_options *options)
options->add_remove = diff_addremove;
options->use_color = diff_use_color_default;
options->detect_rename = diff_detect_rename_default;
+ options->xdl_opts |= diff_algorithm;
if (diff_no_prefix) {
options->a_prefix = options->b_prefix = "";
--
1.8.0.2
^ permalink raw reply related
* [PATCH 3/3] diff: Introduce --diff-algorithm command line option
From: Michal Privoznik @ 2013-01-12 16:02 UTC (permalink / raw)
To: git; +Cc: peff, trast
In-Reply-To: <cover.1358006339.git.mprivozn@redhat.com>
Since command line options have higher priority than config file
variables and taking previous commit into account, we need a way
how to specify myers algorithm on command line. However,
inventing `--myers` is not the right answer. We need far more
general option, and that is `--diff-algorithm`. The older options
(`--minimal`, `--patience` and `--histogram`) are kept for
backward compatibility.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
Documentation/diff-options.txt | 22 ++++++++++++++++++++++
contrib/completion/git-completion.bash | 11 +++++++++++
diff.c | 12 +++++++++++-
diff.h | 2 ++
merge-recursive.c | 6 ++++++
5 files changed, 52 insertions(+), 1 deletion(-)
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 39f2c50..4091f52 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -45,6 +45,9 @@ ifndef::git-format-patch[]
Synonym for `-p --raw`.
endif::git-format-patch[]
+The next three options are kept for compatibility reason. You should use the
+`--diff-algorithm` option instead.
+
--minimal::
Spend extra time to make sure the smallest possible
diff is produced.
@@ -55,6 +58,25 @@ endif::git-format-patch[]
--histogram::
Generate a diff using the "histogram diff" algorithm.
+--diff-algorithm={patience|minimal|histogram|myers}::
+ Choose a diff algorithm. The variants are as follows:
++
+--
+`myers`;;
+ The basic greedy diff algorithm.
+`minimal`;;
+ Spend extra time to make sure the smallest possible diff is
+ produced.
+`patience`;;
+ Use "patience diff" algorithm when generating patches.
+`histogram`;;
+ This algorithm extends the patience algorithm to "support
+ low-occurrence common elements".
+--
++
+You should prefer this option over the `--minimal`, `--patience` and
+`--histogram` which are kept just for backwards compatibility.
+
--stat[=<width>[,<name-width>[,<count>]]]::
Generate a diffstat. By default, as much space as necessary
will be used for the filename part, and the rest for the graph
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 33e25dc..d592cf9 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1021,6 +1021,8 @@ _git_describe ()
__gitcomp_nl "$(__git_refs)"
}
+__git_diff_algorithms="myers minimal patience histogram"
+
__git_diff_common_options="--stat --numstat --shortstat --summary
--patch-with-stat --name-only --name-status --color
--no-color --color-words --no-renames --check
@@ -1035,6 +1037,7 @@ __git_diff_common_options="--stat --numstat --shortstat --summary
--raw
--dirstat --dirstat= --dirstat-by-file
--dirstat-by-file= --cumulative
+ --diff-algorithm=
"
_git_diff ()
@@ -1042,6 +1045,10 @@ _git_diff ()
__git_has_doubledash && return
case "$cur" in
+ --diff-algorithm=*)
+ __gitcomp "$__git_diff_algorithms" "" "${cur##--diff-algorithm=}"
+ return
+ ;;
--*)
__gitcomp "--cached --staged --pickaxe-all --pickaxe-regex
--base --ours --theirs --no-index
@@ -2114,6 +2121,10 @@ _git_show ()
" "" "${cur#*=}"
return
;;
+ --diff-algorithm=*)
+ __gitcomp "$__git_diff_algorithms" "" "${cur##--diff-algorithm=}"
+ return
+ ;;
--*)
__gitcomp "--pretty= --format= --abbrev-commit --oneline
$__git_diff_common_options
diff --git a/diff.c b/diff.c
index ddae5c4..6418076 100644
--- a/diff.c
+++ b/diff.c
@@ -144,7 +144,7 @@ static int git_config_rename(const char *var, const char *value)
return git_config_bool(var,value) ? DIFF_DETECT_RENAME : 0;
}
-static long parse_algorithm_value(const char *value)
+long parse_algorithm_value(const char *value)
{
if (!value || !strcasecmp(value, "myers"))
return 0;
@@ -3633,6 +3633,16 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
options->xdl_opts = DIFF_WITH_ALG(options, PATIENCE_DIFF);
else if (!strcmp(arg, "--histogram"))
options->xdl_opts = DIFF_WITH_ALG(options, HISTOGRAM_DIFF);
+ else if (!prefixcmp(arg, "--diff-algorithm=")) {
+ long value = parse_algorithm_value(arg+17);
+ if (value < 0)
+ return error("option diff-algorithm accepts \"myers\", "
+ "\"minimal\", \"patience\" and \"histogram\"");
+ /* clear out previous settings */
+ DIFF_XDL_CLR(options, NEED_MINIMAL);
+ options->xdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;
+ options->xdl_opts |= value;
+ }
/* flags options */
else if (!strcmp(arg, "--binary")) {
diff --git a/diff.h b/diff.h
index a47bae4..54c2590 100644
--- a/diff.h
+++ b/diff.h
@@ -333,6 +333,8 @@ extern struct userdiff_driver *get_textconv(struct diff_filespec *one);
extern int parse_rename_score(const char **cp_p);
+extern long parse_algorithm_value(const char *value);
+
extern int print_stat_summary(FILE *fp, int files,
int insertions, int deletions);
extern void setup_diff_pager(struct diff_options *);
diff --git a/merge-recursive.c b/merge-recursive.c
index d882060..53d8feb 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -2068,6 +2068,12 @@ int parse_merge_opt(struct merge_options *o, const char *s)
o->xdl_opts = DIFF_WITH_ALG(o, PATIENCE_DIFF);
else if (!strcmp(s, "histogram"))
o->xdl_opts = DIFF_WITH_ALG(o, HISTOGRAM_DIFF);
+ else if (!strcmp(s, "diff-algorithm=")) {
+ long value = parse_algorithm_value(s+15);
+ if (value < 0)
+ return -1;
+ o->xdl_opts |= value;
+ }
else if (!strcmp(s, "ignore-space-change"))
o->xdl_opts |= XDF_IGNORE_WHITESPACE_CHANGE;
else if (!strcmp(s, "ignore-all-space"))
--
1.8.0.2
^ permalink raw reply related
* Re: [PATCH] cvsimport: rewrite to use cvsps 3.x to fix major bugs
From: Eric S. Raymond @ 2013-01-12 16:11 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Junio C Hamano, git, Chris Rorvick
In-Reply-To: <50F17DB0.2050802@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu>:
> Otherwise, how do we know that cvsps currently works with git-cvsimport?
> (OK, you claim that it does, but in the next breath you admit that
> there is a new failure in "one pathological tagging case".) How can we
> understand its strengths/weaknesses? How can we gain confidence that it
> works on different platforms? How will we find out if a future versions
> of cvsps stops working (e.g., because of a breakage or a
> non-backwards-compatible change)?
You can't. But in practice the git crew was going to lose that
capability anyway simply because the new wrapper will support three
engines rather than just one. It's not practical for the git tests to
handle that many variant external dependencies.
However, there is a solution.
The solution is for git to test that the wrapper is *generating the
expected commands*. So what the git tree ends up with is conditional
assurance; the wrapper will do the right thing if the engine it calls
is working correctly. I think that's really all the git-tree tests
can hope for.
Michael, the engines are my problem and yours - it's *our*
responsibility to develop a (hopefully shared) test suite to verify
that they convert repos correctly. I'm working my end as fast as I can;
I hope to have the test suite factored out of cvsps and ready to check
multiple engines by around Wednesday. I still need to convert t9604,
too.
I have parsecvs working since yesterday, so we really are up to three
engines.
I have two minor features I need to merge into parsecvs before
I can start on splitting out the test suite.
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
^ permalink raw reply
* [PATCH 1/3] git-completion.bash: Autocomplete --minimal and --histogram for git-diff
From: Michal Privoznik @ 2013-01-12 16:02 UTC (permalink / raw)
To: git; +Cc: peff, trast
In-Reply-To: <cover.1358006339.git.mprivozn@redhat.com>
Even though --patience was already there, we missed --minimal and
--histogram for some reason.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
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 a4c48e1..383518c 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1031,7 +1031,7 @@ __git_diff_common_options="--stat --numstat --shortstat --summary
--no-ext-diff
--no-prefix --src-prefix= --dst-prefix=
--inter-hunk-context=
- --patience
+ --patience --histogram --minimal
--raw
--dirstat --dirstat= --dirstat-by-file
--dirstat-by-file= --cumulative
--
1.8.0.2
^ permalink raw reply related
* Re: What's cooking in git.git (Jan 2013, #05; Fri, 11)
From: Duy Nguyen @ 2013-01-12 16:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vip739su3.fsf@alter.siamese.dyndns.org>
On Sat, Jan 12, 2013 at 6:56 AM, Junio C Hamano <gitster@pobox.com> wrote:
> * nd/parse-pathspec (2013-01-11) 20 commits
>
> Uses the parsed pathspec structure in more places where we used to
> use the raw "array of strings" pathspec.
>
> Unfortunately, this conflicts a couple of topics in flight. I tried
> to be careful while resolving conflicts, though.
parse_pathspec has not picked up init_pathspec changes from
jk/pathspec-literal and nd/pathspec-wildcard (already in master) so
--literal-pathspecs is probably broken in 'pu' after a lot of
init_pathspec -> parse_pathspec conversion.
--
Duy
^ 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