* 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
* 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: [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: [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
* [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: 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
* 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 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: [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: [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
* [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
* [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 1/3] t/lib-cvs.sh: allow cvsps version 3.x.
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>
Signed-off-by: Chris Rorvick <chris@rorvick.com>
---
t/lib-cvs.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/lib-cvs.sh b/t/lib-cvs.sh
index 44263ad..b55e861 100644
--- a/t/lib-cvs.sh
+++ b/t/lib-cvs.sh
@@ -15,7 +15,7 @@ export CVS
cvsps_version=`cvsps -h 2>&1 | sed -ne 's/cvsps version //p'`
case "$cvsps_version" in
-2.1 | 2.2*)
+2.1 | 2.2* | 3.*)
;;
'')
skip_all='skipping cvsimport tests, cvsps not found'
--
1.8.1.rc3.335.g88a67d6
^ permalink raw reply related
* [PATCH v2 0/3] fixup remaining cvsimport tests
From: Chris Rorvick @ 2013-01-12 6:21 UTC (permalink / raw)
To: git; +Cc: Eric S. Raymond, Junio C Hamano, Chris Rorvick
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(-)
--
1.8.1.rc3.335.g88a67d6
^ permalink raw reply
* Re: [PATCH] t/t960[123]: remove leftover scripts
From: Chris Rorvick @ 2013-01-12 6:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Eric S. Raymond, git
In-Reply-To: <7v1udr9d0g.fsf_-_@alter.siamese.dyndns.org>
On Fri, Jan 11, 2013 at 11:38 PM, Junio C Hamano <gitster@pobox.com> wrote:
> By the way, Chris, we'll need your Sign-off on the three paches
> (t/lib-cvs.sh fix to allow cvsps v3, t9600 fix and t9604 fix).
Sure. I was just maintaining them for myself but thought I'd share
when I saw the follow-up patch. Didn't think to amend them.
Chris
^ permalink raw reply
* Re: [PATCH v2 03/21] Export parse_pathspec() and convert some get_pathspec() calls
From: Duy Nguyen @ 2013-01-12 6:00 UTC (permalink / raw)
To: Matt Kraai, git, Junio C Hamano
In-Reply-To: <20130111175644.GA12359@ftbfs.org>
On Sat, Jan 12, 2013 at 12:56 AM, Matt Kraai <kraai@ftbfs.org> wrote:
> On Fri, Jan 11, 2013 at 06:20:57PM +0700, Nguyễn Thái Ngọc Duy wrote:
>> +#define PATHSPEC_FROMTOP (1<<0)
>
> The previous commit introduces a use of this macro in get_pathspec.
> Should this be defined by that commit instead?
This macro is already defined in setup.c when parse_pathspec is
introduced. I wanted to move it from setup.c to cache.h but forgot to
remove the original definition. Will fix.
>> @@ -266,9 +266,9 @@ static int pathspec_item_cmp(const void *a_, const void *b_)
>> * Given command line arguments and a prefix, convert the input to
>> * pathspec. die() if any magic other than ones in magic_mask.
>> */
>> -static void parse_pathspec(struct pathspec *pathspec,
>> - unsigned magic_mask, unsigned flags,
>> - const char *prefix, const char **argv)
>> +void parse_pathspec(struct pathspec *pathspec,
>> + unsigned magic_mask, unsigned flags,
>
> The prototype for this function uses just "magic" instead of
> "magic_mask". Should they be consistent?
Definitely. Will fix.
--
Duy
^ permalink raw reply
* Re: [PATCH] tests: turn on test-lint-shell-syntax by default
From: Junio C Hamano @ 2013-01-12 6:00 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: git
In-Reply-To: <201301120650.46479.tboegi@web.de>
Torsten Bögershausen <tboegi@web.de> writes:
> The test Makefile has a default set of lint tests which are run
> as part of "make test".
>
> The macro TEST_LINT defaults to "test-lint-duplicates test-lint-executable".
>
> Add test-lint-shell-syntax here, to detect non-portable shell syntax early.
>
> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
> ---
As I said already, I do not want to do this yet without further
reduction of false positives.
Addition of the shell script test was a good starting point, but as
it stands, it still is too brittle and will trigger on something
even trivially innouous, like this:
test_expect_success 'no issues' '
cat >test.file <<-\EOF &&
which is the right way?
EOF
'
> t/Makefile | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/Makefile b/t/Makefile
> index 1923cc1..6fa2b80 100644
> --- a/t/Makefile
> +++ b/t/Makefile
> @@ -13,7 +13,7 @@ TAR ?= $(TAR)
> RM ?= rm -f
> PROVE ?= prove
> DEFAULT_TEST_TARGET ?= test
> -TEST_LINT ?= test-lint-duplicates test-lint-executable
> +TEST_LINT ?= test-lint-duplicates test-lint-executable test-lint-shell-syntax
>
> # Shell quote;
> SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
^ permalink raw reply
* [PATCH] tests: turn on test-lint-shell-syntax by default
From: Torsten Bögershausen @ 2013-01-12 5:50 UTC (permalink / raw)
To: git; +Cc: tboegi
The test Makefile has a default set of lint tests which are run
as part of "make test".
The macro TEST_LINT defaults to "test-lint-duplicates test-lint-executable".
Add test-lint-shell-syntax here, to detect non-portable shell syntax early.
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
t/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/Makefile b/t/Makefile
index 1923cc1..6fa2b80 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -13,7 +13,7 @@ TAR ?= $(TAR)
RM ?= rm -f
PROVE ?= prove
DEFAULT_TEST_TARGET ?= test
-TEST_LINT ?= test-lint-duplicates test-lint-executable
+TEST_LINT ?= test-lint-duplicates test-lint-executable test-lint-shell-syntax
# Shell quote;
SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
--
1.8.0.197.g5a90748
^ permalink raw reply related
* [BUG] Possible bug in `remote set-url --add --push`
From: Jardel Weyrich @ 2013-01-12 5:44 UTC (permalink / raw)
To: git
Hi,
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)
4. Missing an URL? Re-add the original one
jweyrich@pharao:test_clone1 [* master]$ git remote set-url --add --push origin \
/Volumes/sandbox/test
5. Check what happened, again
jweyrich@pharao:test_clone1 [* master]$ git remote -v
origin /Volumes/sandbox/test (fetch)
origin /Volumes/sandbox/test_clone2 (push)
origin /Volumes/sandbox/test (push)
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.
Please, could someone try to reproduce?
- jw
^ permalink raw reply
* [PATCH] t/t960[123]: remove leftover scripts
From: Junio C Hamano @ 2013-01-12 5:38 UTC (permalink / raw)
To: Eric S. Raymond; +Cc: git, Chris Rorvick
In-Reply-To: <7v62339du4.fsf@alter.siamese.dyndns.org>
The rewrite patch was supposed to remove these scripts, but somehow
we ended up removing only the supporting files for them but not the
test script themselves. Remove them for real.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* I'll queue this on top of your patch together with a few fix-up
patches from Chris Rorvick. This may have been caused by your
private patch e-mail mangled somewhere between us before I resent
your patch, or perhaps you simply may have forgot to remove them,
but at this point I do not really care where these deletions were
lost---the only thing I care about is to make sure that you
_meant_ to remove them in your patch (i.e. if you didn't mean to,
then I am further breaking the tests in a way you did not intend
to), so I'd appreciate either "Yup, these three files should be
removed", or "No, they should stay; removal of their supporting
data is no longer needed" from you (I and this patch expect the
former, of course).
By the way, Chris, we'll need your Sign-off on the three paches
(t/lib-cvs.sh fix to allow cvsps v3, t9600 fix and t9604 fix).
t/t9601-cvsimport-vendor-branch.sh | 85 --------------------------------------
t/t9602-cvsimport-branches-tags.sh | 78 ----------------------------------
t/t9603-cvsimport-patchsets.sh | 39 -----------------
3 files changed, 202 deletions(-)
delete mode 100755 t/t9601-cvsimport-vendor-branch.sh
delete mode 100755 t/t9602-cvsimport-branches-tags.sh
delete mode 100755 t/t9603-cvsimport-patchsets.sh
diff --git a/t/t9601-cvsimport-vendor-branch.sh b/t/t9601-cvsimport-vendor-branch.sh
deleted file mode 100755
index 827d39f..0000000
--- a/t/t9601-cvsimport-vendor-branch.sh
+++ /dev/null
@@ -1,85 +0,0 @@
-#!/bin/sh
-
-# Description of the files in the repository:
-#
-# imported-once.txt:
-#
-# Imported once. 1.1 and 1.1.1.1 should be identical.
-#
-# imported-twice.txt:
-#
-# Imported twice. HEAD should reflect the contents of the
-# second import (i.e., have the same contents as 1.1.1.2).
-#
-# imported-modified.txt:
-#
-# Imported, then modified on HEAD. HEAD should reflect the
-# modification.
-#
-# imported-modified-imported.txt:
-#
-# Imported, then modified on HEAD, then imported again.
-#
-# added-imported.txt,v:
-#
-# Added with 'cvs add' to create 1.1, then imported with
-# completely different contents to create 1.1.1.1, therefore the
-# vendor branch was never the default branch.
-#
-# imported-anonymously.txt:
-#
-# Like imported-twice.txt, but with a vendor branch whose branch
-# tag has been removed.
-
-test_description='git cvsimport handling of vendor branches'
-. ./lib-cvs.sh
-
-setup_cvs_test_repository t9601
-
-test_expect_success PERL 'import a module with a vendor branch' '
-
- git cvsimport -C module-git module
-
-'
-
-test_expect_success PERL 'check HEAD out of cvs repository' 'test_cvs_co master'
-
-test_expect_success PERL 'check master out of git repository' 'test_git_co master'
-
-test_expect_success PERL 'check a file that was imported once' '
-
- test_cmp_branch_file master imported-once.txt
-
-'
-
-test_expect_failure PERL 'check a file that was imported twice' '
-
- test_cmp_branch_file master imported-twice.txt
-
-'
-
-test_expect_success PERL 'check a file that was imported then modified on HEAD' '
-
- test_cmp_branch_file master imported-modified.txt
-
-'
-
-test_expect_success PERL 'check a file that was imported, modified, then imported again' '
-
- test_cmp_branch_file master imported-modified-imported.txt
-
-'
-
-test_expect_success PERL 'check a file that was added to HEAD then imported' '
-
- test_cmp_branch_file master added-imported.txt
-
-'
-
-test_expect_success PERL 'a vendor branch whose tag has been removed' '
-
- test_cmp_branch_file master imported-anonymously.txt
-
-'
-
-test_done
diff --git a/t/t9602-cvsimport-branches-tags.sh b/t/t9602-cvsimport-branches-tags.sh
deleted file mode 100755
index e1db323..0000000
--- a/t/t9602-cvsimport-branches-tags.sh
+++ /dev/null
@@ -1,78 +0,0 @@
-#!/bin/sh
-
-# A description of the repository used for this test can be found in
-# t9602/README.
-
-test_description='git cvsimport handling of branches and tags'
-. ./lib-cvs.sh
-
-setup_cvs_test_repository t9602
-
-test_expect_success PERL 'import module' '
-
- git cvsimport -C module-git module
-
-'
-
-test_expect_success PERL 'test branch master' '
-
- test_cmp_branch_tree master
-
-'
-
-test_expect_success PERL 'test branch vendorbranch' '
-
- test_cmp_branch_tree vendorbranch
-
-'
-
-test_expect_failure PERL 'test branch B_FROM_INITIALS' '
-
- test_cmp_branch_tree B_FROM_INITIALS
-
-'
-
-test_expect_failure PERL 'test branch B_FROM_INITIALS_BUT_ONE' '
-
- test_cmp_branch_tree B_FROM_INITIALS_BUT_ONE
-
-'
-
-test_expect_failure PERL 'test branch B_MIXED' '
-
- test_cmp_branch_tree B_MIXED
-
-'
-
-test_expect_success PERL 'test branch B_SPLIT' '
-
- test_cmp_branch_tree B_SPLIT
-
-'
-
-test_expect_failure PERL 'test tag vendortag' '
-
- test_cmp_branch_tree vendortag
-
-'
-
-test_expect_success PERL 'test tag T_ALL_INITIAL_FILES' '
-
- test_cmp_branch_tree T_ALL_INITIAL_FILES
-
-'
-
-test_expect_failure PERL 'test tag T_ALL_INITIAL_FILES_BUT_ONE' '
-
- test_cmp_branch_tree T_ALL_INITIAL_FILES_BUT_ONE
-
-'
-
-test_expect_failure PERL 'test tag T_MIXED' '
-
- test_cmp_branch_tree T_MIXED
-
-'
-
-
-test_done
diff --git a/t/t9603-cvsimport-patchsets.sh b/t/t9603-cvsimport-patchsets.sh
deleted file mode 100755
index 52034c8..0000000
--- a/t/t9603-cvsimport-patchsets.sh
+++ /dev/null
@@ -1,39 +0,0 @@
-#!/bin/sh
-
-# Structure of the test cvs repository
-#
-# Message File:Content Commit Time
-# Rev 1 a: 1.1 2009-02-21 19:11:43 +0100
-# Rev 2 a: 1.2 b: 1.1 2009-02-21 19:11:14 +0100
-# Rev 3 b: 1.2 2009-02-21 19:11:43 +0100
-#
-# As you can see the commit of Rev 3 has the same time as
-# Rev 1 this leads to a broken import because of a cvsps
-# bug.
-
-test_description='git cvsimport testing for correct patchset estimation'
-. ./lib-cvs.sh
-
-setup_cvs_test_repository t9603
-
-test_expect_failure 'import with criss cross times on revisions' '
-
- git cvsimport -p"-x" -C module-git module &&
- (cd module-git &&
- git log --pretty=format:%s > ../actual-master &&
- git log A~2..A --pretty="format:%s %ad" -- > ../actual-A &&
- echo "" >> ../actual-master &&
- echo "" >> ../actual-A
- ) &&
- echo "Rev 4
-Rev 3
-Rev 2
-Rev 1" > expect-master &&
- test_cmp actual-master expect-master &&
-
- echo "Rev 5 Branch A Wed Mar 11 19:09:10 2009 +0000
-Rev 4 Branch A Wed Mar 11 19:03:52 2009 +0000" > expect-A &&
- test_cmp actual-A expect-A
-'
-
-test_done
--
1.8.1.421.g6236851
^ permalink raw reply related
* Re: [PATCH] cvsimport: rewrite to use cvsps 3.x to fix major bugs
From: Junio C Hamano @ 2013-01-12 5:20 UTC (permalink / raw)
To: Eric S. Raymond; +Cc: git
In-Reply-To: <1357875152-19899-1-git-send-email-gitster@pobox.com>
I cloned git://gitorious.org/cvsps/cvsps.git and installed cvsps-3.7
at c2ce6cc (More fun with test loads, sigh. Timezones suck.,
2013-01-09) earlier on my $PATH, and tried to run t96xx series with
this patch applied on top of Git 1.8.1.
The first thing I noticed was that all the tests were skipped.
A patch to t/lib-cvs.sh might be sufficient,
--------------------- >8 -------------------------
diff --git a/t/lib-cvs.sh b/t/lib-cvs.sh
index 44263ad..423953f 100644
--- a/t/lib-cvs.sh
+++ b/t/lib-cvs.sh
@@ -17,6 +17,8 @@ cvsps_version=`cvsps -h 2>&1 | sed -ne 's/cvsps version //p'`
case "$cvsps_version" in
2.1 | 2.2*)
;;
+3.*)
+ ;;
'')
skip_all='skipping cvsimport tests, cvsps not found'
test_done
--------------------- 8< -------------------------
but I didn't check more than "now it seems not to skip them".
And here is what I got:
--------------------- >8 -------------------------
Test Summary Report
-------------------
t9600-cvsimport.sh (Wstat: 256 Tests: 15 Failed: 9)
Failed tests: 4-6, 8-9, 11-13, 15
Non-zero exit status: 1
t9601-cvsimport-vendor-branch.sh (Wstat: 256 Tests: 9 Failed: 8)
Failed tests: 1-4, 6-9
Non-zero exit status: 1
t9602-cvsimport-branches-tags.sh (Wstat: 256 Tests: 11 Failed: 5)
Failed tests: 1-3, 7, 9
Non-zero exit status: 1
t9604-cvsimport-timestamps.sh (Wstat: 256 Tests: 2 Failed: 2)
Failed tests: 1-2
Non-zero exit status: 1
Files=5, Tests=38, 5 wallclock secs ( 0.05 usr 0.01 sys + 0.49
cusr 0.16 csys = 0.71 CPU)
Result: FAIL
--------------------- 8< -------------------------
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. 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?
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.
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?
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.
If the updated cvsps can identify the changeset more reliably and it
no longer needs "-a" option, it may be more helpful to the users to
migrate their script if it allowed, warned and then ignored the
option. It certainly would help sharing of this test script between
runs that use the old and new cvsps as backends.
t9601 (after resurrecting the t/t9601/cvsroot directory) fails in an
interesting way.
--------------------- >8 -------------------------
$ sh t9601-cvsimport-vendor-branch.sh -i -v
Initialized empty Git repository in /git/git.build/t/trash directory.t9601-cvsimport-vendor-branch/.git/
expecting success:
git cvsimport -C module-git module
Traceback (most recent call last):
File "/git/git.build/git-cvsimport", line 262, in <module>
subprocess.check_output("cvsps -V 2>/dev/null", shell=True)
AttributeError: 'module' object has no attribute 'check_output'
not ok - 1 import a module with a vendor branch
--------------------- 8< -------------------------
Apparently, the copy of "subprocess.py" I have does not give us the
check_output thing:
--------------------- >8 -------------------------
$ python
Python 2.6.6 (r266:84292, Dec 26 2010, 22:31:48)
[GCC 4.4.5] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import subprocess
>>> dir(subprocess)
['CalledProcessError', 'MAXFD', 'PIPE', 'Popen', 'STDOUT', '__all__', '__builtins__', '__doc__', '__file__', '__name__', '__package__', '_active', '_cleanup', '_demo_posix', '_demo_windows', '_eintr_retry_call', 'call', 'check_call', 'errno', 'fcntl', 'gc', 'list2cmdline', 'mswindows', 'os', 'pickle', 'select', 'signal', 'sys', 'traceback', 'types']
--------------------- 8< -------------------------
The story is the same for t9602 and t9603 (again after resurrecting
the necessary files).
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.
^ permalink raw reply related
* Re: [PATCH] cvsimport: rewrite to use cvsps 3.x to fix major bugs
From: Eric S. Raymond @ 2013-01-12 5:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmwwfbjv3.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com>:
> Yeah, it is OK to _discourage_ its use, but to me it looks like that
> the above is a fairly subjective policy decision, not something I
> should let you impose on the users of the old cvsimport, which you
> do not seem to even treat as your users.
Er. You still don't seem to grasp the fundamentals of this
situation. I'm not imposing any damn thing on the users. What's
imposing is the fact that cvsps-2.x and the Perl cvsimport are both
individually and collectively *broken right now*, and within a few
months the Perl git-cvsimport is going to cease even pretending to
work. I'm trying to *fix that problem* as best I can, fixing it
required two radical rewrites, and criticizing me for not emulating
every last detail and misfeature immediately is every bit as pointless
and annoying as arguing about the fabric on the deck chairs while the
ship is sinking.
To put it bluntly, you should be grateful to be getting back any
functionality at all - because the alternative is that the Perl
git-cvsimport will hang out in your tree as a dead piece of cruft.
Your choice is between making it easy for me replace it with minimum
disruption now and hoping for someone else to replace it months from
now after you've had a bunch of unhappy users bitching at you.
So let me be more direct. I think the -M and -m options are
sufficiently bad ideas that I am *not willing* to put in the quite
large amount of effort that would be required to implement them in cvsps
or parsecvs. That would be a bad use of my time.
This is not the case with -o; that might be a good idea if I
understood it. This is also not like the 2.x fallback; I thought that
was a bad idea (because it would be better for users that the
combination break in an obvious way than continue breaking in a silent
one), but it was a small enough effort that I was willing to do it
anyway to keep the git maintainer happy. The effort to fix the quoting
bugs is even easier for me to justify; they are actual bugs.
Those are my engineering judgments; go ahead and call them
"subjective" if you like, but neither the facts nor my judgment will
change on that account.
> The "major" in my sentence was from your description (the bugs you
> fixed), and not about the new ones you still have in this draft. I
> did not mean to say that you are trading fixes to "major" bugs with
> different "major" bugs.
OK, thank you. In the future I will try to bear in mind that English
is not your primary language when I evaluate statements that seems a bit
offensive.
So what's your next bid? Note that you can't increase my friction and
hassle costs much more before I give up and let you deal with the
consequences without me. I want to do the right thing, but I have
more other projects clamoring for my attention than you could easily
guess. I need to get git-cvsimport *finished* and off my hands -
I may already have given it more time than I really should have.
So give me your minimum list of deliverables before you'll merge,
please, and then stick to it. I assume fixes for the quoting bugs
will be on that list.
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
^ permalink raw reply
* [PATCH v2] t9605: test for cvsps commit ordering bug
From: Chris Rorvick @ 2013-01-12 4:39 UTC (permalink / raw)
To: git; +Cc: Eric S. Raymond, Chris Rorvick
Import of a trivial CVS repository fails due to a cvsps bug. Given the
following series of commits:
timestamp a b c message
------------------- --- --- --- -------
2012/12/12 21:09:39 1.1 changes are done
2012/12/12 21:09:44 1.1 changes
2012/12/12 21:09:46 1.2 changes
2012/12/12 21:09:50 1.1 1.3 changes are done
cvsps mangles the commit ordering (edited for brevity):
---------------------
PatchSet 1
Date: 2012/12/12 15:09:39
Log:
changes are done
Members:
a:INITIAL->1.1
b:INITIAL->1.1
c:1.2->1.3
---------------------
PatchSet 2
Date: 2012/12/12 15:09:44
Log:
changes
Members:
c:INITIAL->1.1
---------------------
PatchSet 3
Date: 2012/12/12 15:09:46
Log:
changes
Members:
c:1.1->1.2
This is seen in cvsps versions 2.x and up through at least 3.7.
Signed-off-by: Chris Rorvick <chris@rorvick.com>
---
It actually does fail without the "&& false" at the end. :-P Sorry for
the noise.
t/t9605-cvsimport-commit-order.sh | 24 +++++++++++++++
t/t9605/cvsroot/.gitattributes | 1 +
t/t9605/cvsroot/CVSROOT/.gitignore | 2 ++
t/t9605/cvsroot/module/a,v | 24 +++++++++++++++
t/t9605/cvsroot/module/b,v | 24 +++++++++++++++
t/t9605/cvsroot/module/c,v | 62 ++++++++++++++++++++++++++++++++++++++
6 files changed, 137 insertions(+)
create mode 100755 t/t9605-cvsimport-commit-order.sh
create mode 100644 t/t9605/cvsroot/.gitattributes
create mode 100644 t/t9605/cvsroot/CVSROOT/.gitignore
create mode 100644 t/t9605/cvsroot/module/a,v
create mode 100644 t/t9605/cvsroot/module/b,v
create mode 100644 t/t9605/cvsroot/module/c,v
diff --git a/t/t9605-cvsimport-commit-order.sh b/t/t9605-cvsimport-commit-order.sh
new file mode 100755
index 0000000..86aafd1
--- /dev/null
+++ b/t/t9605-cvsimport-commit-order.sh
@@ -0,0 +1,24 @@
+#!/bin/sh
+
+test_description='git cvsimport commit order'
+. ./lib-cvs.sh
+
+setup_cvs_test_repository t9605
+
+test_expect_success 'checkout with CVS' '
+
+ echo CVSROOT=$CVSROOT &&
+ cvs checkout -d module-cvs module
+'
+
+test_expect_failure 'import into git (commit order mangled)' '
+
+ git cvsimport -R -a -p"-x" -C module-git module &&
+ (
+ cd module-git &&
+ git merge origin
+ ) &&
+ test_cmp module-cvs/c module-git/c
+'
+
+test_done
diff --git a/t/t9605/cvsroot/.gitattributes b/t/t9605/cvsroot/.gitattributes
new file mode 100644
index 0000000..562b12e
--- /dev/null
+++ b/t/t9605/cvsroot/.gitattributes
@@ -0,0 +1 @@
+* -whitespace
diff --git a/t/t9605/cvsroot/CVSROOT/.gitignore b/t/t9605/cvsroot/CVSROOT/.gitignore
new file mode 100644
index 0000000..3bb9b34
--- /dev/null
+++ b/t/t9605/cvsroot/CVSROOT/.gitignore
@@ -0,0 +1,2 @@
+history
+val-tags
diff --git a/t/t9605/cvsroot/module/a,v b/t/t9605/cvsroot/module/a,v
new file mode 100644
index 0000000..6455911
--- /dev/null
+++ b/t/t9605/cvsroot/module/a,v
@@ -0,0 +1,24 @@
+head 1.1;
+access;
+symbols;
+locks; strict;
+comment @# @;
+
+
+1.1
+date 2012.12.12.21.09.39; author tester; state Exp;
+branches;
+next ;
+
+
+desc
+@@
+
+
+1.1
+log
+@changes are done
+@
+text
+@file a
+@
diff --git a/t/t9605/cvsroot/module/b,v b/t/t9605/cvsroot/module/b,v
new file mode 100644
index 0000000..55545c8
--- /dev/null
+++ b/t/t9605/cvsroot/module/b,v
@@ -0,0 +1,24 @@
+head 1.1;
+access;
+symbols;
+locks; strict;
+comment @# @;
+
+
+1.1
+date 2012.12.12.21.09.50; author tester; state Exp;
+branches;
+next ;
+
+
+desc
+@@
+
+
+1.1
+log
+@changes are done
+@
+text
+@file b
+@
diff --git a/t/t9605/cvsroot/module/c,v b/t/t9605/cvsroot/module/c,v
new file mode 100644
index 0000000..d3eac77
--- /dev/null
+++ b/t/t9605/cvsroot/module/c,v
@@ -0,0 +1,62 @@
+head 1.3;
+access;
+symbols;
+locks; strict;
+comment @# @;
+
+
+1.3
+date 2012.12.12.21.09.50; author tester; state Exp;
+branches;
+next 1.2;
+
+1.2
+date 2012.12.12.21.09.46; author tester; state Exp;
+branches;
+next 1.1;
+
+1.1
+date 2012.12.12.21.09.44; author tester; state Exp;
+branches;
+next ;
+
+
+desc
+@@
+
+
+1.3
+log
+@changes are done
+@
+text
+@file c
+line two
+line three
+line four
+line five
+@
+
+
+1.2
+log
+@changes
+@
+text
+@d2 4
+a5 4
+line 2
+line 3
+line 4
+line 5
+@
+
+
+1.1
+log
+@changes
+@
+text
+@d2 4
+@
+
--
1.8.1.rc3.335.g88a67d6
^ permalink raw reply related
* [PATCH] t9605: test for cvsps commit ordering bug
From: Chris Rorvick @ 2013-01-12 4:13 UTC (permalink / raw)
To: git; +Cc: Eric S. Raymond, Chris Rorvick
Import of a trivial CVS repository fails due to a cvsps bug. Given the
following series of commits:
timestamp a b c message
------------------- --- --- --- -------
2012/12/12 21:09:39 1.1 changes are done
2012/12/12 21:09:44 1.1 changes
2012/12/12 21:09:46 1.2 changes
2012/12/12 21:09:50 1.1 1.3 changes are done
cvsps mangles the commit ordering (edited for brevity):
---------------------
PatchSet 1
Date: 2012/12/12 15:09:39
Log:
changes are done
Members:
a:INITIAL->1.1
b:INITIAL->1.1
c:1.2->1.3
---------------------
PatchSet 2
Date: 2012/12/12 15:09:44
Log:
changes
Members:
c:INITIAL->1.1
---------------------
PatchSet 3
Date: 2012/12/12 15:09:46
Log:
changes
Members:
c:1.1->1.2
This is seen in cvsps versions 2.x and up through at least 3.7.
Signed-off-by: Chris Rorvick <chris@rorvick.com>
---
Ran into this recently. No branching and no "criss cross" timestamps,
just lazy commit messages. And it magically backed out a bug fix.
This applies on top of master. With minor modifications I've tested it
with Eric's latest code and confirmed the bug still exists.
Chris
t/t9605-cvsimport-commit-order.sh | 25 +++++++++++++++
t/t9605/cvsroot/.gitattributes | 1 +
t/t9605/cvsroot/CVSROOT/.gitignore | 2 ++
t/t9605/cvsroot/module/a,v | 24 +++++++++++++++
t/t9605/cvsroot/module/b,v | 24 +++++++++++++++
t/t9605/cvsroot/module/c,v | 62 ++++++++++++++++++++++++++++++++++++++
6 files changed, 138 insertions(+)
create mode 100755 t/t9605-cvsimport-commit-order.sh
create mode 100644 t/t9605/cvsroot/.gitattributes
create mode 100644 t/t9605/cvsroot/CVSROOT/.gitignore
create mode 100644 t/t9605/cvsroot/module/a,v
create mode 100644 t/t9605/cvsroot/module/b,v
create mode 100644 t/t9605/cvsroot/module/c,v
diff --git a/t/t9605-cvsimport-commit-order.sh b/t/t9605-cvsimport-commit-order.sh
new file mode 100755
index 0000000..ab4042e
--- /dev/null
+++ b/t/t9605-cvsimport-commit-order.sh
@@ -0,0 +1,25 @@
+#!/bin/sh
+
+test_description='git cvsimport commit order'
+. ./lib-cvs.sh
+
+setup_cvs_test_repository t9605
+
+test_expect_success 'checkout with CVS' '
+
+ echo CVSROOT=$CVSROOT &&
+ cvs checkout -d module-cvs module
+'
+
+test_expect_failure 'import into git (commit order mangled)' '
+
+ git cvsimport -R -a -p"-x" -C module-git module &&
+ (
+ cd module-git &&
+ git merge origin
+ ) &&
+ test_cmp module-cvs/c module-git/c &&
+false
+'
+
+test_done
diff --git a/t/t9605/cvsroot/.gitattributes b/t/t9605/cvsroot/.gitattributes
new file mode 100644
index 0000000..562b12e
--- /dev/null
+++ b/t/t9605/cvsroot/.gitattributes
@@ -0,0 +1 @@
+* -whitespace
diff --git a/t/t9605/cvsroot/CVSROOT/.gitignore b/t/t9605/cvsroot/CVSROOT/.gitignore
new file mode 100644
index 0000000..3bb9b34
--- /dev/null
+++ b/t/t9605/cvsroot/CVSROOT/.gitignore
@@ -0,0 +1,2 @@
+history
+val-tags
diff --git a/t/t9605/cvsroot/module/a,v b/t/t9605/cvsroot/module/a,v
new file mode 100644
index 0000000..6455911
--- /dev/null
+++ b/t/t9605/cvsroot/module/a,v
@@ -0,0 +1,24 @@
+head 1.1;
+access;
+symbols;
+locks; strict;
+comment @# @;
+
+
+1.1
+date 2012.12.12.21.09.39; author tester; state Exp;
+branches;
+next ;
+
+
+desc
+@@
+
+
+1.1
+log
+@changes are done
+@
+text
+@file a
+@
diff --git a/t/t9605/cvsroot/module/b,v b/t/t9605/cvsroot/module/b,v
new file mode 100644
index 0000000..55545c8
--- /dev/null
+++ b/t/t9605/cvsroot/module/b,v
@@ -0,0 +1,24 @@
+head 1.1;
+access;
+symbols;
+locks; strict;
+comment @# @;
+
+
+1.1
+date 2012.12.12.21.09.50; author tester; state Exp;
+branches;
+next ;
+
+
+desc
+@@
+
+
+1.1
+log
+@changes are done
+@
+text
+@file b
+@
diff --git a/t/t9605/cvsroot/module/c,v b/t/t9605/cvsroot/module/c,v
new file mode 100644
index 0000000..d3eac77
--- /dev/null
+++ b/t/t9605/cvsroot/module/c,v
@@ -0,0 +1,62 @@
+head 1.3;
+access;
+symbols;
+locks; strict;
+comment @# @;
+
+
+1.3
+date 2012.12.12.21.09.50; author tester; state Exp;
+branches;
+next 1.2;
+
+1.2
+date 2012.12.12.21.09.46; author tester; state Exp;
+branches;
+next 1.1;
+
+1.1
+date 2012.12.12.21.09.44; author tester; state Exp;
+branches;
+next ;
+
+
+desc
+@@
+
+
+1.3
+log
+@changes are done
+@
+text
+@file c
+line two
+line three
+line four
+line five
+@
+
+
+1.2
+log
+@changes
+@
+text
+@d2 4
+a5 4
+line 2
+line 3
+line 4
+line 5
+@
+
+
+1.1
+log
+@changes
+@
+text
+@d2 4
+@
+
--
1.8.1.rc3.335.g88a67d6
^ permalink raw reply related
* Re: git send-email should not allow 'y' for in-reply-to
From: Junio C Hamano @ 2013-01-12 2:56 UTC (permalink / raw)
To: Ben Aveling
Cc: Antoine Pelisse, Jeff King, Matt Seitz (matseitz),
git@vger.kernel.org, Hilco Wijbenga
In-Reply-To: <50F0B643.20201@optusnet.com.au>
Ben Aveling <bena.001@optusnet.com.au> writes:
> On 12/01/2013 10:54 AM, Junio C Hamano wrote:
>> Antoine Pelisse <apelisse@gmail.com> writes:
>>
>>> I would simply go for:
>>>
>>> What Message-ID are you replying to (if any)?
>>>
>>> If I don't know what to answer, I would definitely not say y/yes/n/no,
>>> but press enter directly.
>> Sounds sensible (even though technically you reply to a message
>> that has that message ID, and not to a message ID ;-)).
>>
>> Any better phrasing from others? If not, I'd say we adopt this
>> text.
>
> I guess it depends on how much we mind if people accidentally miss the
> message ID.
>
> If we don't mind much, we could say something like:
>
> What Message-ID are you replying to [Default=None]?
>
>
> If we are concerned that when a Message-ID exists, it should be
> provided, we could split to 2 questions:
>
> Are you replying to an existing Message [Y/n]?
>
> And then, if the answer is Y,
>
> What Message-ID are you replying to?
Eewww. Now we come back to full circles.
It sometimes helps to follow the in-reply-to chain to see what has
already been said in the thread, I guess ;-)
^ 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