Hi,
I've fixed the git-svn tests for SVN 1.7 and tested with SVN 1.7.5. SVN 1.7
changed its expectations of path and URL formats and git-svn did not comply
with them. The new code uses SVN's own canonicalization routines where
available. This has been reported in several places...
On 2012.7.17 10:44 AM, Jonathan Nieder wrote:
Michael G Schwern wrote:
I've fixed the git-svn tests for SVN 1.7 and tested with SVN 1.7.5.
Thanks. git-svn is not maintained by Junio but by Eric and others on
the list. I'm cc-ing Eric and Ben Walton so they can benefit from
your work
From 683a230e439f1d5ac2727ce4c2a74e93804fc72b Mon Sep 17 00:00:00 2001
From: Michael G. Schwern schw...@pobox.com
Date: Wed, 11 Jul 2012 22:16:01 -0700
Subject: [PATCH 03/11] Fix Git::SVN so it can at least compile alone.
It's still very intertwined with git-svn, but that's a lot of work
From 5f0b609e9b0a70c86c46b48f0b180c96c3355a14 Mon Sep 17 00:00:00 2001
From: Michael G. Schwern schw...@pobox.com
Date: Tue, 17 Jul 2012 15:40:03 -0700
Subject: [PATCH 04/11] Extract Git::SVN::Log from git-svn.
This is a straight cut paste. Next commit will make it work. This will
make
From 8f70be0424a770c299b6a0c5bf99e4030e5e4d92 Mon Sep 17 00:00:00 2001
From: Michael G. Schwern schw...@pobox.com
Date: Thu, 12 Jul 2012 16:58:53 -0700
Subject: [PATCH 05/11] Make Git::SVN::Log work.
Changes to Git::SVN::Log to make it compile
* Change the $_git_format lexical only used
From ab67ab421dbfd248b9a198b8cc1cd9944ba6178d Mon Sep 17 00:00:00 2001
From: Michael G. Schwern schw...@pobox.com
Date: Tue, 17 Jul 2012 15:46:44 -0700
Subject: [PATCH 06/11] Move Git::SVN::Migration into its own file.
Just a straight cut paste, the fixes come next commit.
---
git-svn.perl
From cb1a73929da15e87fa3dcc41c4cfa9ca592081fa Mon Sep 17 00:00:00 2001
From: Michael G. Schwern schw...@pobox.com
Date: Thu, 12 Jul 2012 17:14:24 -0700
Subject: [PATCH 07/11] Fix Git::SVN::Migration after its move.
Also...
* eliminate the big import all the Git command functions loop, nothing
From 9ff49d9e91c9741d501620ac47f78d8ff8ef9983 Mon Sep 17 00:00:00 2001
From: Michael G. Schwern schw...@pobox.com
Date: Tue, 17 Jul 2012 15:51:53 -0700
Subject: [PATCH 08/11] Cut paste Git::IndexInfo into its own file.
No other changes, those are next commit so they can be seen in the diff
From 4fd7b8574b32753dcf22ec0a592f13586b938689 Mon Sep 17 00:00:00 2001
From: Michael G. Schwern schw...@pobox.com
Date: Thu, 12 Jul 2012 17:20:02 -0700
Subject: [PATCH 09/11] Fix Git::IndexInfo so it compiles.
Only used by Git::SVN::Fetcher.
---
perl/Git/IndexInfo.pm | 2 ++
perl/Git/SVN
From 368d6c7883080d85f82b1eae1815834e3d59ef5e Mon Sep 17 00:00:00 2001
From: Michael G. Schwern schw...@pobox.com
Date: Tue, 17 Jul 2012 15:54:33 -0700
Subject: [PATCH 10/11] Cut paste Git::SVN::GlobSpec into its own file.
Fixes to make it work come next commit.
---
git-svn.perl
From 5152b76800f076ba0bd528664f62d3c67966fa4e Mon Sep 17 00:00:00 2001
From: Michael G. Schwern schw...@pobox.com
Date: Thu, 12 Jul 2012 17:25:25 -0700
Subject: [PATCH 11/11] Fix Git::SVN::GlobSpec so it works.
Only used in one place in Git::SVN, load it on demand.
That should be all the Git
On 2012.7.17 5:01 PM, Jonathan Nieder wrote:
It also moves Error.pm into a bundle directory. This both makes it just
another directory to scan (or not scan), but it also makes it possible to
bundle additional modules in the future. ExtUtils::MakeMaker uses this
technique itself.
This is
On 2012.7.18 3:58 AM, Eric Wong wrote:
I agree with everything Jonathan said (and thank him for taking
the time to point you in the right direction).
Thanks, you guys have been very nice to my flailing and failing. I'm going to
back off and send out a sort of overview email so we can figure
It's post OSCON so I can take another crack at this again.
I'm struggling with how best to present all this to you folks. There's
etiquette for how one presents a git pull request... but there's conflicting
etiquette about how one presents patches to a mailing list. I'm not sure
which bit of
On 2012.7.17 10:49 PM, Junio C Hamano wrote:
By allowing people to easily publish a completed work, and making it
easier for them to let others peek at their work, Git hosting
services like GitHub are wonderful. But I am not conviced that
quality code reviews like we do on the mailing list
On 2012.7.24 2:51 PM, Junio C Hamano wrote:
Michael G Schwern schw...@pobox.com writes:
A big one is do not blast 10 emails to a mailing list but I gather that's
ok
here if a submission needs 10 commits to be well expressed and its done via
git-send-email? And then if patch #3 needs
On 2012.7.24 4:45 PM, Junio C Hamano wrote:
git log -p schwern/git-svn/extract-classes..schwern/git-svn/fix-canonical
That should give you the information you need...
I guess so. May we have your sign-off on these changes? (A simple
reply of yes is enough, no need to resend patches to do
This makes it so you no longer must edit the Makefile.PL every time you
add, rename or delete a Perl module. This is convenient, and I'm about
to extract a bunch of .pm files out of git-svn.
You still have to edit the Makefile. That parallel build system should be
able to be removed at a later
From: Michael G. Schwern schw...@pobox.com
Usually it isn't, but its nice if it can be run with warnings on.
Signed-off-by: Michael G Schwern schw...@pobox.com
---
perl/Makefile.PL | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/perl/Makefile.PL b/perl/Makefile.PL
index
From: Michael G. Schwern schw...@pobox.com
In older Perls, sometimes $@ can become unset between the eval and
checking $@. Its safer to check the eval directly.
Signed-off-by: Michael G Schwern schw...@pobox.com
---
perl/Makefile.PL | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff
From: Michael G. Schwern schw...@pobox.com
It is no longer necessary to manually add new .pm files to the
Makefile.PL. This makes it easier to add modules.
It is still necessary to add them to the Makefile, but that extra work
should be removed at a future date.
Signed-off-by: Michael G
On 2012.7.24 9:53 PM, Jonathan Nieder wrote:
Michael G Schwern wrote:
No, now it's just canonicalizing as early as possible. Preferably within the
object accessor rather than at the point of use. So in the code below,
$full_url is already escaped/canonicalized.
Let's start
On 2012.7.24 7:55 PM, Eric Wong wrote:
After I'm exhausted from volunteering all the coding work, rather than
submitting a URL to a remote repository I find I have to learn new
specialized
tools. It's extra learning and work, an extra step to screw up, and foreign
to me (even as a
This is a refactoring to move Git::SVN out of git-svn and into its own .pm file.
This will make it easier to work with and test. This is just the extraction
with minimal work to keep all tests passing.
A couple of utility functions which were used by Git::SVN, git-svn and others
were also
From: Michael G. Schwern schw...@pobox.com
This means it should be able to load without git-svn being loaded.
* Load Git.pm on its own and all the needed command functions.
* It needs to grab at a git-svn lexical $_prefix representing the --prefix
option. Provide opt_prefix
From: Michael G. Schwern schw...@pobox.com
Also it can compile on its own now, yay!
---
git-svn.perl | 4
perl/Git/SVN.pm | 9 +++--
t/Git-SVN/00compile.t | 3 ++-
3 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index 4c77f69
On 2012.7.24 10:54 PM, Jonathan Nieder wrote:
And again, it *does not have to be zero sum*. It doesn't have to be email VS
GUI. You can have your cake and eat it too.
I assume you're talking about web-based interfaces that have gateways
to email, that produce inboxes like this:
24 Jul
On 2012.7.25 12:14 AM, Junio C Hamano wrote:
Nothing, because paths are not URI escaped. :)
You probably meant svn_uri_canonicalize(). And no, it does not double
escape,
so its safe to escape as early as possible.
Are you saying that the function assumes that a local pathname would
On 2012.7.25 9:56 AM, Junio C Hamano wrote:
Looks sensible. Will queue.
Thanks.
Thanks!
What's the lag time on it showing up in the repo, and which branch will it
appear in?
Also I just realized I've been basing my work on master. Should I move to
maint?
--
If you want the truth to
On 2012.7.25 2:15 PM, Jonathan Nieder wrote:
Michael G. Schwern wrote:
This is a refactoring to move Git::SVN out of git-svn and into its own .pm
file.
This will make it easier to work with and test. This is just the extraction
with minimal work to keep all tests passing.
Patch 3 doesn't
On 2012.7.25 3:19 PM, Junio C Hamano wrote:
Michael G Schwern schw...@pobox.com writes:
How about the git-svn SVN 1.7 fix in general? All of these patch sets I'm
sending build on one another, is that going to be a problem? It's going to
come in about six parts.
Judging from the rate
This is rapidly getting into the weeds. Regardless of the debate below, what
would you like me to do? Switch indentation to tabs and resubmit? AFAIK only
the new tests are affected.
On 2012.7.25 4:08 PM, Eric Wong wrote:
+BEGIN {
+# Override exit at BEGIN time before Git::SVN::Utils is
Same as before, now with tab indentation in the new Perl tests.
As before, patch #3 is 132k and will be rejected by some of the lists.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at
From: Michael G. Schwern schw...@pobox.com
Put them in a new module called Git::SVN::Utils. Yeah, not terribly
original and it will be a dumping ground. But its better than having
them in the main git-svn program. At least they can be documented
and tested.
* fatal() is used by many classes
From: Michael G. Schwern schw...@pobox.com
Also it can compile on its own now, yay!
---
git-svn.perl | 4
perl/Git/SVN.pm | 9 +++--
t/Git-SVN/00compile.t | 3 ++-
3 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index 4c77f69
This series of patches extracts the remaining classes from git-svn. They're
all simple extractions and functionally have no change.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at
From: Michael G. Schwern schw...@pobox.com
* Load Git command functions itself.
* Can't access the git-svn switch lexical any more, but its only used by
Git::SVN::Log so turn it into a Git::SVN::Log global.
* Load Git::SVN as needed. No need to load it always, its only used twice.
* Moved
From: Michael G. Schwern schw...@pobox.com
Straight cut paste.
Also noticed Git::SVN::Ra wasn't in the compile test. It is now.
---
git-svn.perl | 395 +-
perl/Git/SVN/Log.pm | 395
From: Michael G. Schwern schw...@pobox.com
---
git-svn.perl | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index 584e93a..4d173d4 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -31,14 +31,14 @@ use Git::SVN::Migration;
use Git
From: Michael G. Schwern schw...@pobox.com
Straight cut paste. That's the last class.
* Make Git::SVN load it on its own, its the only thing that needs it.
---
git-svn.perl | 59
perl/Git/SVN.pm | 2 ++
perl/Git/SVN
From: Michael G. Schwern schw...@pobox.com
Straight cut paste.
---
git-svn.perl | 258 +-
perl/Git/SVN/Migration.pm | 258 ++
perl/Makefile | 1 +
t/Git-SVN/00compile.t | 3
From: Michael G. Schwern schw...@pobox.com
Just makes the code easier to follow. No functional change.
Also eliminate an unused lexical $SVN.
---
git-svn.perl | 44 +---
1 file changed, 21 insertions(+), 23 deletions(-)
diff --git a/git-svn.perl b/git
On 2012.7.26 10:18 PM, Junio C Hamano wrote:
Forgot to sign-off, or are you still unsure about this step?
I just never think to do it. It's just a line in the commit message, right?
There's no crypto involved like tag -s. Is it a blocker? I guess I can write
a msg-filter if it's important.
On 2012.7.26 10:18 PM, Junio C Hamano wrote:
If you swap the order of steps 3/4 and 4/4 by creating Git/SVN.pm
that only has these variable definitions (i.e. our $X and use
vars $X) and make git-svn.perl use them from Git::SVN in the first
step, and then do the bulk-moving (equivalent of your
This patch series gives Git::SVN and Git::SVN::Ra accessors for
path and url and then makes the rest of the code use them, rather
than grab at $obj-{path} and $obj-{url}. This then will give
us the control necessary to canonicalize them as early as
possible (done in the next patch series).
There
From: Michael G. Schwern schw...@pobox.com
Then later it can be canonicalized automatically rather than everywhere
its used.
Later patch will make other things use it.
---
perl/Git/SVN.pm | 87 +
1 file changed, 56 insertions(+), 31
From: Michael G. Schwern schw...@pobox.com
So later it can do automatic canonicalization.
A later patch will make other things use the accessor.
No functional change here.
---
perl/Git/SVN.pm | 44 ++--
1 file changed, 30 insertions(+), 14 deletions
From: Michael G. Schwern schw...@pobox.com
No functional change.
---
git-svn.perl| 12 +++-
perl/Git/SVN.pm | 4 ++--
perl/Git/SVN/Fetcher.pm | 2 +-
perl/Git/SVN/Ra.pm | 6 +++---
4 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/git-svn.perl b
From: Michael G. Schwern schw...@pobox.com
Note: The structure returned from Git::SVN-read_all_remotes() does not appear
to
contain objects, so I'm leaving them alone.
That's everything converted over to the url and path accessors.
No functional change.
---
git-svn.perl | 11
On 2012.7.27 1:07 PM, Eric Wong wrote:
While Makefile.PL now finds .pm files on its own, it does not
detect new files after it generates perl/perl.mak.
Are you saying this doesn't work?
perl Makefile.PL
make -f perl.mak
touch Git/Foo.pm
perl Makefile.PL
make -f perl.mak
or this?
perl
On 2012.7.27 8:10 PM, Jonathan Nieder wrote:
This is the last refactoring patch series. After this bugs, start
getting fixed.
I just wanted to say thanks for your thoughtful presentation of this
code. I was worried before, but these have been pleasantly submitted.
You're welcome. I've
From: Michael G. Schwern schw...@pobox.com
So they can be used by others.
I'd like to test them, but they're going to become SVN API wrappers shortly
and those aren't predictable.
No functional change.
---
git-svn.perl | 33 +++-
perl/Git/SVN/Utils.pm | 52
From: Michael G. Schwern schw...@pobox.com
The SVN API functions will not accept ../foo but their canonicalization
functions will not collapse it. So we'll have to do it ourselves.
_collapse_dotdot() works better than the existing regex did.
This will be used shortly when canonicalize_path
From: Michael G. Schwern schw...@pobox.com
The code doesn't use File::Spec.
---
perl/Git/SVN/Utils.pm | 2 --
1 file changed, 2 deletions(-)
diff --git a/perl/Git/SVN/Utils.pm b/perl/Git/SVN/Utils.pm
index deade07..6c8ae53 100644
--- a/perl/Git/SVN/Utils.pm
+++ b/perl/Git/SVN/Utils.pm
@@ -92,8
From: Michael G. Schwern schw...@pobox.com
All tests pass with SVN 1.6. SVN 1.7 remains broken, not worrying
about it yet.
SVN changed its path canonicalization API between 1.6 and 1.7.
http://svnbook.red-bean.com/en/1.6/svn.developer.usingapi.html#svn.developer.usingapi.urlpath
http
From: Michael G. Schwern schw...@pobox.com
This canonicalizes paths and urls as early as possible so we don't
have to remember to do it at the point of use. It will fix a swath
of SVN 1.7 problems in one go.
Its ok to double canonicalize things.
SVN 1.7 still fails, still not worrying about
This patch series fixes git-svn for SVN 1.7 tested against SVN 1.7.5 and
1.6.18. Patch 7/8 is where SVN 1.7 starts passing.
There is one exception. t9100-git-svn-basic.sh fails 11-13. This appears
to be due to a bug in SVN to do with symlinks. Leave that for somebody
else, this is the final
From: Michael G. Schwern schw...@pobox.com
Test to check that the migration got rid of the old style git-svn directory.
It wasn't failing, just throwing a message to STDERR.
---
t/t9107-git-svn-migrate.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t9107-git-svn
From: Michael G. Schwern schw...@pobox.com
Continuing to move towards getting everything canonicalizing the same way.
* Git::SVN-init_remote_config and Git::SVN::Ra-minimize_url both
have to canonicalize the same way else init_remote_config
will incorrectly think they're different URLs
From: Michael G. Schwern schw...@pobox.com
Rather than guess what SVN is going to do for each version, make the test use
the branch name that was actually created.
---
t/t9118-git-svn-funky-branch-names.sh | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/t/t9118-git-svn
From: Michael G. Schwern schw...@pobox.com
---
git-svn.perl| 8
perl/Git/SVN.pm | 1 -
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index 3d120d5..56d1ba7 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -36,6 +36,7 @@ use Git::SVN::Utils
From: Michael G. Schwern schw...@pobox.com
Remove the ad-hoc versions.
This is mostly to normalize the process and ensure the URLs produced
don't have double slashes or anything.
Also provides a place to fix the corner case where a file path
contains a percent sign.
---
git-svn.perl
From: Michael G. Schwern schw...@pobox.com
Go through all the spots that use the new add_path_to_url() to
make a new URL and canonicalize them.
* copyfrom_path has to be canonicalized else find_parent_branch
will get confused
* due to the `canonicalize_url($full_url) ne $full_url)` line
From: Michael G. Schwern schw...@pobox.com
Previously, our URL canonicalization didn't do much of anything.
Now it actually escapes and collapses slashes. This is mostly a cut paste
of escape_url from git-svn.
This is closer to how SVN 1.7's canonicalization behaves. Doing it with
1.6 lets us
On 2012.7.28 6:50 AM, Jonathan Nieder wrote:
--- a/perl/Git/SVN/Utils.pm
+++ b/perl/Git/SVN/Utils.pm
[...]
@@ -100,6 +102,20 @@ API as a URL.
=cut
sub canonicalize_url {
+my $url = shift;
+
+# The 1.7 way to do it
+if ( defined SVN::_Core::svn_uri_canonicalize ) {
+
On 2012.7.28 6:55 AM, Jonathan Nieder wrote:
Michael G. Schwern wrote:
--- a/perl/Git/SVN/Utils.pm
+++ b/perl/Git/SVN/Utils.pm
@@ -86,6 +86,27 @@ sub _collapse_dotdot {
sub canonicalize_path {
+my $path = shift;
+
+# The 1.7 way to do it
+if ( defined SVN::_Core
On 2012.7.28 7:11 AM, Jonathan Nieder wrote:
Yay! Am I correct in imagining this makes the following sequence of
commands[1] no longer trip an assertion failure in svn_path_join[2]
with SVN 1.6?
git svn init -Thttp://trac-hacks.org/svn/tagsplugin/trunk \
On 2012.7.28 7:16 AM, Jonathan Nieder wrote:
Michael G. Schwern wrote:
Rather than guess what SVN is going to do for each version, make the test use
the branch name that was actually created.
[...]
-git rev-parse refs/remotes/not-a%40{0}reflog
+git rev-parse refs
On 2012.7.28 12:30 PM, Jonathan Nieder wrote:
I didn't know about that. I don't know what your SVN backwards compat
requirements are, but if that behavior goes back far enough in SVN to satisfy
you folks, then canonicalize_url() should fall back to
SVN::_Core::svn_path_canonicalize().
On 2012.7.28 1:02 PM, Jonathan Nieder wrote:
Jonathan Nieder wrote:
Michael G Schwern wrote:
I would suggest that worrying whether a few lines of code are introduced now
or 10 patches later in the same branch which is all going to be merged in
one
go (and retesting the patches after
On 2012.7.30 12:51 PM, Eric Wong wrote:
The SVN API functions will not accept ../foo but their canonicalization
functions will not collapse it. So we'll have to do it ourselves.
_collapse_dotdot() works better than the existing regex did.
I don't dispute it's better, but it's worth
On 2012.7.30 1:38 PM, Eric Wong wrote:
A better solution would be to have path and URL objects which overload
the eq operator and automatically stringify canonicalized and escaped.
Perhaps we can depend on the URI.pm module? It seems to be
widely-available and not be a significant barrier
On 2012.7.30 3:15 PM, Eric Wong wrote:
Right now, canonicalization is a bug generator. Paths and URLs have to be in
the same form when they're compared. This requires meticulous care on the
part of the coder and reviewer to check every comparison. It scatters the
logic for proper comparison
On 2012.7.30 7:18 PM, Eric Wong wrote:
Michael G Schwern schw...@pobox.com wrote:
On 2012.7.30 3:15 PM, Eric Wong wrote:
Right now, canonicalization is a bug generator. Paths and URLs have to be
in
the same form when they're compared. This requires meticulous care on the
part of the coder
It just doesn't matter.
Why are we arguing over which solution will be 4% better two years from now,
or if my commits are formatted perfectly, when tremendous amounts of basic
work to be done improving git-svn? The code is undocumented, lacking unit
tests, difficult to understand and riddled
On 2012.7.31 1:01 PM, Eric Wong wrote:
Michael G Schwern schw...@pobox.com wrote:
It just doesn't matter.
Why are we arguing over which solution will be 4% better two years from now,
or if my commits are formatted perfectly, when tremendous amounts of basic
work to be done improving git-svn
On 2012.7.31 4:05 PM, Junio C Hamano wrote:
What I won't accept is maintainability does not matter. It does.
I'm sorry, that's not what I intended to convey at all. My reply to Eric lays
it out more clearly, I think.
--
Reality is that which, when you stop believing in it, doesn't go away.
On 2012.8.2 2:51 PM, Eric Wong wrote:
svn_path_canonicalize() may be accessible in some versions of SVN,
but it'll return undef.
Yuck! Good catch!
I've tested the following on an old CentOS 5.2 chroot with SVN 1.4.2:
Looks good to me.
--
Alligator sandwich, and make it snappy!
--
To
78 matches
Mail list logo
