Return-Path: X-Original-To: apmail-subversion-dev-archive@minotaur.apache.org Delivered-To: apmail-subversion-dev-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id A6BC3119DE for ; Thu, 28 Aug 2014 11:19:20 +0000 (UTC) Received: (qmail 10488 invoked by uid 500); 28 Aug 2014 11:19:20 -0000 Delivered-To: apmail-subversion-dev-archive@subversion.apache.org Received: (qmail 10434 invoked by uid 500); 28 Aug 2014 11:19:20 -0000 Mailing-List: contact dev-help@subversion.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Delivered-To: mailing list dev@subversion.apache.org Received: (qmail 10424 invoked by uid 99); 28 Aug 2014 11:19:20 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 28 Aug 2014 11:19:20 +0000 X-ASF-Spam-Status: No, hits=1.5 required=5.0 tests=HTML_MESSAGE,RCVD_IN_DNSWL_LOW,SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (nike.apache.org: domain of stefan.fuhrmann@wandisco.com designates 209.85.213.182 as permitted sender) Received: from [209.85.213.182] (HELO mail-ig0-f182.google.com) (209.85.213.182) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 28 Aug 2014 11:18:54 +0000 Received: by mail-ig0-f182.google.com with SMTP id a13so708754igq.3 for ; Thu, 28 Aug 2014 04:18:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=wandisco.com; s=gapps; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=ypKYZGH+D6VRxJ54HMrxD9zskfvSf0XWyBWhzqfrZEU=; b=bSiRTtK32Zi57shoErMNJKvkDTY+vsUrqIYr2Y2ENWnMztyEGUY9IN2tghvMvEJ0+k IrhLTv1LJR9j+iig/txM5Lus/kpkLow6X0+ljjQ5kLzBIuEliSEqanXN0NVLbnkZ1wLx aACYNBLxOw50IoPrmFSmcwnEpxh58ol3dc0d0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=ypKYZGH+D6VRxJ54HMrxD9zskfvSf0XWyBWhzqfrZEU=; b=K7MrEK7eMR3RMzfIE4rGKWCahrNsFk6OlkfjL7GFJhrAjQK9GGoibgGwxIL26p+lTh phehe8IiEpyWHELPotetNHcyCpMIScRLg/qApRi6Efs6faHWK4dEdp/XleDoMlgfbGOG 0HxjRoACF+nAh6Gy0Si5TIZsbwN82oWd/GjnuckhWJsoGZdBKjQ4m7KMnmGxwQunov47 hZkNXxu7Yq3zo2459GggWOVhUfSv42mbtmZHxBZiPX8c1h86rBZCSRQm3hV8MtT8Ouei Qx4iEYkt999LJudRvaYwGsnX4p/CTqVFD6buW1qx1pFxvuUYQsdy6WCdOWPidnTkNzRE 1b0Q== X-Gm-Message-State: ALoCoQnT1Qi1Q1hwZfN0rwsIsGPdwNB1det4NkgKCr6bfqSj1vhK5QXNEGXEAk4APrqlP81XIAIp MIME-Version: 1.0 X-Received: by 10.50.103.106 with SMTP id fv10mr2491340igb.40.1409224732554; Thu, 28 Aug 2014 04:18:52 -0700 (PDT) Received: by 10.50.176.165 with HTTP; Thu, 28 Aug 2014 04:18:52 -0700 (PDT) In-Reply-To: References: <53FC92F9.70408@collab.net> Date: Thu, 28 Aug 2014 13:18:52 +0200 Message-ID: Subject: Re: Moving some of our tools to "main" subversion From: Stefan Fuhrmann To: Ivan Zhakov Cc: "C. Michael Pilato" , Subversion Development Content-Type: multipart/alternative; boundary=047d7b3a8a5e97d3780501aeb3c6 X-Virus-Checked: Checked by ClamAV on apache.org --047d7b3a8a5e97d3780501aeb3c6 Content-Type: text/plain; charset=UTF-8 On Wed, Aug 27, 2014 at 5:54 PM, Ivan Zhakov wrote: > On 27 August 2014 19:42, Stefan Fuhrmann > wrote: > > On Tue, Aug 26, 2014 at 4:00 PM, C. Michael Pilato > > wrote: > >> > >> On 08/26/2014 09:02 AM, Stefan Fuhrmann wrote: > > > > > >> > >> > Note that we > >> > never include such headers in current Subversion code except > >> > "fs-loader.h" and tests. > >> > > >> > > >> > Would moving the declarations (2 structs, 10 functions) > >> > to a new "include/private/svn_fs_fs_private.h" be sufficient > >> > in your opinion? > >> > >> I should think that would be sufficient. But then, it wasn't my opinion > >> that you solicited. :-) > > > > > > Implemented in r1620909. > > > Stefan, > > This is completely wrong approach. What problem are you trying to solve? > Please revert immediately. > > Proper way is to implement three specific semi-private functions in > libsvn_fs_fs for collecting stats, dumping and loading FS indexes and > then use in svnfsfs. > Sounds like a good idea at the first glance, but it * adds a large new API (instead of moving existing declarations) * requires new code and various changes to existing code * may require a separate library (which would defeat the whole purpose) * can be done partly or entirely at any point in the future (no guarantees made that could be broken) So, there is no reason to do it just now and there are a few minor points against it - considering that we will be branching 1.9 shortly. The remainder of this post only goes into more detail on some of the above. This new set of API would still be fully private. Nothing for which I would want to provide stability guarantees. If, at some point in the future, we should provide more comprehensive and mature reporting and recovery functionality, then there may be point to provide a stable API plus bindings for integration into some admin environment. Right now, this new API would still export the index data structure, i.e. duplicate structures and #defines as well as add code to convert between "internal" and "external" API. The current internal API based code is only about 20 lines each for "load index" and "dump index". Everything else is UI formatting and input parsing. For the stats code, things are slightly different. Here, the progress and results API would be quite complex (mainly the result data structures) and involve lots of code churn (naming conventions, callbacks etc). I'm also not fully convinced that such code should go into the FSFS core library. One may make the point for the index access (it's recovery-ish) but the stats code? That's data processing and I feel it should be kept separate from the data provider module, i.e. in a libsvn_client-esque library. My current opinion about that is not too strong. But I can see this functionality grow in later releases, possibly adding entirely new features. All that would bloat the core back-end. -- Stefan^2. --047d7b3a8a5e97d3780501aeb3c6 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
On W= ed, Aug 27, 2014 at 5:54 PM, Ivan Zhakov <ivan@visualsvn.com> wrote:
On 27 August 2014 19:42, Stefan Fu= hrmann <stefan.fuhrmann@wandisco.com> wrote:
> On Tue, Aug 26, 2014 at 4:00 PM, C. Michael Pilato <cmpilato@collab.net>
> wrote:
>>
>> On 08/26/2014 09:02 AM, Stefan Fuhrmann wrote:
>
>
>>
>> >=C2=A0 =C2=A0 =C2=A0Note that we
>> >=C2=A0 =C2=A0 =C2=A0never include such headers in current Subv= ersion code except
>> >=C2=A0 =C2=A0 =C2=A0"fs-loader.h" and tests.
>> >
>> >
>> > Would moving the declarations (2 structs, 10 functions)
>> > to a new "include/private/svn_fs_fs_private.h" be s= ufficient
>> > in your opinion?
>>
>> I should think that would be sufficient.=C2=A0 But then, it wasn&#= 39;t my opinion
>> that you solicited. :-)
>
>
> Implemented in r1620909.
>
Stefan,

This is completely wrong approach.

What pro= blem are you trying to solve?
=C2=A0
Please revert immediately.

Proper way is to implement three specific semi-private functions in
libsvn_fs_fs for collecting stats, dumping and loading FS indexes and
then use in svnfsfs.

Sounds like a good= idea at the first glance, but it

* adds a large new API = (instead of moving existing declarations)
* requires new code= and various changes to existing code
* may require a separate library (which would defeat the whole p= urpose)
* can be done partly or entirely at any point in the = future (no guarantees
=C2=A0 made that could be broken)
So, there is no reason to do it just now and there are a few min= or points
against it - considering that we will be branching 1.9 shortly= . The remainder
of this post only goes into more detail on some of the a= bove.

This new set of API would still = be fully private. Nothing for which I would
want to provide stability gu= arantees. If, at some point in the future, we
should provide more compre= hensive and mature reporting and recovery
functionality, then there may be point to provide a stable API plus binding= s
for integration into some admin envir= onment.

Right now, this new API would still export the index data structure, i.e.duplicate structures and #defines as well as add code to convert between<= br>
"internal" and "external= " API. The current internal API based code is only
about 20 lines each for "load index&q= uot; and "dump index". Everything else
is UI formatting and in= put parsing.

For the stats code, th= ings are slightly different. Here, the progress and
results API would be quite complex (mainly the result data structures)
a= nd involve lots of code churn (naming conventions, callbacks etc).

<= /div>
I'm also not fully convinced that such = code should go into the FSFS core
library. One may make the point for the index access (it's recovery-ish= )
but the stats code? That's data p= rocessing and I feel it should be kept
separate from the data provider m= odule, i.e. in a libsvn_client-esque
library. My current opinion about that is not too strong. But I can see thi= s
functionality grow in later releases, possibly adding entirely new fea= tures.
All that would bloat the core back-end.

-- Stefan^2.
--047d7b3a8a5e97d3780501aeb3c6--