[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Remove svn_wc_translated_* ?

From: Greg Stein <gstein_at_gmail.com>
Date: Mon, 5 Jul 2010 17:18:30 -0400

On Mon, Jul 5, 2010 at 12:35, Hyrum K. Wright
<hyrum_wright_at_mail.utexas.edu> wrote:
> On Mon, Jul 5, 2010 at 10:26 AM, Julian Foad <julian.foad_at_wandisco.com> wrote:
>> On Mon, 2010-07-05, Hyrum K. Wright wrote:
>>> Julian's r960620 prompted me to do a bit of digging in the client
>>> library.  It turns out there is nowhere we use
>>> svn_wc_translated_stream3() and svn_wc_translated_file2(), which are

Torch them!

>...
>>> used to allow the client library to grab the keywords- and
>>> eol-style-translated versions of a file.  Are these APIs now obsolete?
>>>  Can they be removed (the most recent versions are new in 1.7)?  Do
>>> other API consumers depend on them?
>>
>> This doesn't impact directly on whether we should remove the latest
>> (unused) versions and deprecate the previous versions of the APIs you
>> mentioned, but ...
>>
>> I have been thinking the set of "translation" API functions is a bit
>> ragged.  I would like to see a smaller set of API functions with more
>> distinct tasks, something like just the following pair:

"a bit ragged" ... ha! The survivors are a bit ragged. I deprecated a
half-dozen of these damned things before we shipped 1.6. Maybe some
since then. Dunno. But I spent a tortured month revamping these
functions and rewriting a bunch of callpoints to trim down the
hojillion variants that we had. Each one slightly different from the
other. Slightly different inputs. Slightly different semantics.
Torturous.

>>  (A) a libsvn_subr API that translates a given stream according to a
>> given set of properties, not implicitly referring to a versioned node.
>>
>>  (B) a libsvn_wc API that grabs, from a versioned file, the set of
>> properties necessary for translating it (note that this includes at
>> least the rev, author, and date as well as the versioned properties
>> svn:eol-style, svn:mime-type and svn:special);
>>
>> The purpose is to be able to translate arbitrary stored texts -
>> especially conflict left/right/mine files stored in the pristine store -
>> to and from Repository Normal Form without referring to a current
>> versioned node at the time of translation.

Just no adding more variants unless you delete one. Part of the
problem we had was that everybody thought their variant was Better, so
we ended up with too many.

> Yep, reducing the coupling here would be good.  I also ran across this
> little gem in translate.c:
>
>      /* ### ugh. the repair behavior does NOT match the docstring. bleah.
>         ### all of these translation functions are crap and should go
>         ### away anyways. we'll just deprecate most of the functions and
>         ### properly document the survivors */
>
> Apparently the author was thinking along the same lines as you are now. :)

"the author" == me :-P

> Another API anomaly I noticed was that translate_file() and
> translate_stream() have two very different implementations.  In my
> mind, translate file would just wrap translate stream, but it appears
> there is more magic going on here that meets the eye.

I ran out of steam. There is still more work that can be done.

But you should have seen some of the old crap in there! Translate a C
string would create a stringbuf, then wrap a stream around that, then
create a second stringbuf stream, wrap a translator around one, then
stream copy between the two, then fetch the result from that second
stringbuf. Done! ... I rewrote it to throw out that initial wrap and
just svn_stream_write into the translating stream.

Silliness like that abounded :-(

(which is also why I got on Julian's case this past week when we set
up a similar stream copy instead of just doing a simple
svn_stream_write).

>...

Cheers,
-g
Received on 2010-07-05 23:19:08 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.