git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: Fix git-svn for SVN 1.7

From
Michael G Schwern <schwern@pobox.com>
Date
Jul 30, 2012, 21:10 UTC
Message-ID
<5016F832.7030604@pobox.com>
In-Reply-To
<20120730203844.GA23892@dcvr.yhbt.net>
On 2012.7.30 1:38 PM, Eric Wong wrote:
Show 7 quoted lines
>> 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 to installation.  On
> the other hand, I don't know its history, either (especially since we're
> now dealing with SVN changes...).

If you want to go down the road of having CPAN dependencies, then it should definitely be used rather than rolling our own and generating our own bugs. It's a very commonly needed Perl module.

You'd make a subclass and put any special work arounds for SVN in there.
> Anyways, I don't like relying on operator overloading, it makes code
> harder to read and review.

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 all over the code. Redundant logic scattered around the code is a Bad Thing. It makes it more likely a coder will forget the logic, or get it wrong, and a human reviewer must be far more vigilant.

Right now I'm pretty sure there's still a ton of bugs.

It also slows things down. As strings, URLs and paths have to be canonicalized every time they're used or compared. An object representing the URI or path can cache the canonicalization.

With string comparison overloaded, you'd no longer have to meticulously check that URLs and paths are always in the same form when they're compared. It just does it. The logic is in one place. We don't even have to care if one of them is a string (or which one), it works even if only one half of the comparison is an object. A new coder to the project doesn't need to know anything special about URIs and paths, they just treat them as strings. Finally, they can be slipped into existing code without having to rewrite everything.

Overloaded comparison and stringification can even be used as a tool to find all the places in the code where URLs and paths are being used, where they're being turned into strings, and where URL and path manipulation is being done ad-hoc. For example, if comparison sees one of its arguments as a string, or if concatenation is used.

The only downside is when chasing down a bug related to canonicalization one might have to realize that eq is overloaded. But we'd have far less bugs due to canonicalization. So worth it.

-- 
Being faith-based doesn't trump reality.
	-- Bruce Sterling
Previous: Eric WongNext: Eric Wong
Message 26 of 50 in “Fix git-svn for SVN 1.7”
  1. Michael G. SchwernJul 28, 2012
  2. 1/8 SVN 1.7 will truncate "not-a%40{0}" to just "not-a".Michael G. Schwern, Jul 28, 2012
  3. Jonathan NiederJul 28, 2012
  4. Michael G SchwernJul 28, 2012
  5. svn test: escape peg revision separator using empty peg revJonathan Nieder, Oct 9, 2012
  6. Michael J GruberOct 9, 2012
  7. Jonathan NiederOct 9, 2012
  8. Eric WongOct 10, 2012
  9. Jonathan NiederOct 10, 2012
  10. Eric WongOct 10, 2012
  11. Jonathan NiederOct 10, 2012
  12. Eric WongOct 10, 2012
  13. Junio C HamanoOct 10, 2012
  14. 2/8 Fix typo in testMichael G. Schwern, Jul 28, 2012
  15. 3/8 Improve our URL canonicalization to be more like SVN 1.7's.Michael G. Schwern, Jul 28, 2012
  16. 4/8 Replace hand rolled URL escapes with canonicalizationMichael G. Schwern, Jul 28, 2012
  17. 5/8 Canonicalize earlier in a couple spots.Michael G. Schwern, Jul 28, 2012
  18. 6/8 Add function to append a path to a URL.Michael G. Schwern, Jul 28, 2012
  19. 7/8 Turn on canonicalization on newly minted URLs.Michael G. Schwern, Jul 28, 2012
  20. test: work around SVN 1.7 mishandling of svn:special changesJonathan Nieder, Oct 6, 2012
  21. git svn: work around SVN 1.7 mishandling of svn:special changesJonathan Nieder, Oct 9, 2012
  22. Eric WongOct 10, 2012
  23. git svn: work around SVN 1.7 mishandling of svn:special changesJonathan Nieder, Oct 10, 2012
  24. 8/8 Remove some ad hoc canonicalizations.Michael G. Schwern, Jul 28, 2012
  25. Eric WongJul 30, 2012
  26. Michael G SchwernJul 30, 2012
  27. Eric WongJul 30, 2012
  28. Michael G SchwernJul 31, 2012
  29. Eric WongJul 31, 2012
  30. Michael G SchwernJul 31, 2012
  31. Junio C HamanoJul 31, 2012
  32. Michael G SchwernJul 31, 2012
  33. Eric WongJul 31, 2012
  34. Junio C HamanoJul 31, 2012
  35. Michael G SchwernJul 31, 2012
  36. Michael G SchwernJul 31, 2012
  37. Eric WongAug 1, 2012
  38. Eric WongAug 2, 2012
  39. Jonathan NiederAug 2, 2012
  40. Junio C HamanoAug 2, 2012
  41. Robin H. JohnsonAug 2, 2012
  42. Eric WongAug 2, 2012
  43. Junio C HamanoAug 21, 2012
  44. Eric WongAug 21, 2012
  45. Junio C HamanoAug 21, 2012
  46. Eric WongAug 2, 2012
  47. Junio C HamanoAug 2, 2012
  48. Eric WongAug 2, 2012
  49. Eric WongAug 2, 2012
  50. Junio C HamanoAug 2, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.