Return-Path: 
 <commits-return-31142-apmail-subversion-commits-archive=subversion.apache.org@subversion.apache.org>
X-Original-To: apmail-subversion-commits-archive@minotaur.apache.org
Delivered-To: apmail-subversion-commits-archive@minotaur.apache.org
Received: from mail.apache.org (hermes.apache.org [140.211.11.3])
	by minotaur.apache.org (Postfix) with SMTP id 7BCD810E86
	for <apmail-subversion-commits-archive@minotaur.apache.org>;
 Tue,  9 Jul 2013 14:48:35 +0000 (UTC)
Received: (qmail 32850 invoked by uid 500); 9 Jul 2013 14:48:35 -0000
Delivered-To: apmail-subversion-commits-archive@subversion.apache.org
Received: (qmail 32788 invoked by uid 500); 9 Jul 2013 14:48:34 -0000
Mailing-List: contact commits-help@subversion.apache.org; run by ezmlm
Precedence: bulk
List-Help: <mailto:commits-help@subversion.apache.org>
List-Unsubscribe: <mailto:commits-unsubscribe@subversion.apache.org>
List-Post: <mailto:commits@subversion.apache.org>
List-Id: <commits.subversion.apache.org>
Reply-To: dev@subversion.apache.org
Delivered-To: mailing list commits@subversion.apache.org
Received: (qmail 32761 invoked by uid 99); 9 Jul 2013 14:48:33 -0000
Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230)
    by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 09 Jul 2013 14:48:33 +0000
X-ASF-Spam-Status: No, hits=-0.7 required=5.0
	tests=RCVD_IN_DNSWL_LOW
X-Spam-Check-By: apache.org
Received-SPF: error (nike.apache.org: local policy)
Received: from [209.85.212.177] (HELO mail-wi0-f177.google.com)
 (209.85.212.177)
    by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 09 Jul 2013 14:48:26 +0000
Received: by mail-wi0-f177.google.com with SMTP id ey16so5191115wid.10
        for <commits@subversion.apache.org>;
 Tue, 09 Jul 2013 07:47:46 -0700 (PDT)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
        d=google.com; s=20120113;
        h=from:to:cc:references:in-reply-to:subject:date:message-id
         :mime-version:content-type:content-transfer-encoding:x-mailer
         :thread-index:content-language:x-gm-message-state;
        bh=XWJPgxayTVYacltPK1QPbHiOOhhSFxz4BSe0pjxeWRM=;
        b=gUnq6P8nL4cmBQfIdQnN0LAzn9DsYCJetq+Rc/8ZyRx8/I/HJInuda/GNtqHb3T4cD
         pi0TFMRvZwUSZ1a4levk8wxwzldiyg9VVmRbzvC0aNEYD/Mps89sAT8GHeBQ4lgH5/iQ
         bFAGJ5N28006gHhNEf+gwIX11X4cBWUkqonAkFv14GoPe4GwgeNCmbTZH/2y7Z+BdSHH
         xVwvAeBtl5Ex2KSMeMmhP94plLW+vaX9RGapKsILDKK0zLlAwZhXX/zffnesLquQ5phA
         obDY3kVWBgNv0jFYuxQzCeTl0WtkB2H+hY1+31ztYDsc4AJIwEivY0tJ8R2D9cbLxhGQ
         fSAg==
X-Received: by 10.180.83.68 with SMTP id o4mr32574639wiy.5.1373381266715;
        Tue, 09 Jul 2013 07:47:46 -0700 (PDT)
Received: from i72600 ([2001:610:66e:0:59f6:6194:a164:dbd7])
        by mx.google.com with ESMTPSA id m6sm28054786wiy.8.2013.07.09.07.47.44
        for <multiple recipients>
        (version=TLSv1.2 cipher=RC4-SHA bits=128/128);
        Tue, 09 Jul 2013 07:47:45 -0700 (PDT)
From: "Bert Huijben" <bert@qqmail.nl>
To: "'Daniel Shahaf'" <d.s@daniel.shahaf.name>
Cc: <dev@subversion.apache.org>,
	<commits@subversion.apache.org>
References: <20130709023750.8FA4C23888CD@eris.apache.org>
 <015701ce7c9e$b3250d70$196f2850$@qqmail.nl>
 <20130709132721.GA4075@lp-shahaf.local>
 <015e01ce7ca9$1ae0a410$50a1ec30$@qqmail.nl>
 <20130709141223.GC4075@lp-shahaf.local>
In-Reply-To: <20130709141223.GC4075@lp-shahaf.local>
Subject: RE: svn commit: r1501049 - in
 /subversion/trunk/subversion:	include/svn_error_codes.h
 libsvn_ra_serf/util_error.c
Date: Tue, 9 Jul 2013 16:47:36 +0200
Message-ID: <017301ce7cb3$41a62200$c4f26600$@qqmail.nl>
MIME-Version: 1.0
Content-Type: text/plain;
	charset="us-ascii"
Content-Transfer-Encoding: 7bit
X-Mailer: Microsoft Outlook 15.0
Thread-Index: AQC5M1iaHIdlEjrBOhU3XDgzkFxeLwEuY28yAirGwNkBZWZrwAIfSuegm1AQH5A=
Content-Language: nl
X-Gm-Message-State: 
 ALoCoQm5xle85fNdfs88JTnZraLovCrzowu4fZ9++GSlXfarPgFok7I+2dIV2p8jvPbeiHNCNpi8
X-Virus-Checked: Checked by ClamAV on apache.org



> -----Original Message-----
> From: 'Daniel Shahaf' [mailto:d.s@daniel.shahaf.name]
> Sent: dinsdag 9 juli 2013 16:12
> To: Bert Huijben
> Cc: dev@subversion.apache.org; commits@subversion.apache.org
> Subject: Re: svn commit: r1501049 - in /subversion/trunk/subversion:
> include/svn_error_codes.h libsvn_ra_serf/util_error.c
> 
> Bert Huijben wrote on Tue, Jul 09, 2013 at 15:34:56 +0200:
> >
> >
> > > -----Original Message-----
> > > From: Daniel Shahaf [mailto:d.s@daniel.shahaf.name]
> > > Sent: dinsdag 9 juli 2013 15:27
> > > To: Bert Huijben
> > > Cc: dev@subversion.apache.org; commits@subversion.apache.org
> > > Subject: Re: svn commit: r1501049 - in /subversion/trunk/subversion:
> > > include/svn_error_codes.h libsvn_ra_serf/util_error.c
> > >
> > > Can you please give me a little credit?  The problem here is not that
> > > 120171 doesn't get stringified in maintainer mode.
> > >
> > > The problem here is that it is a number which _does not have_ a
symbolic
> > > name in APR's or Subversion's API, so if an API user wants to code
> > > against it they need to either hard-code the number or #include
<serf.h>
> > > in their builds --- and last I checked, we didn't require API users to
> > > do either of those.  Compare this to sqlite: we wrap all SQLite error
> > > integers to SVN_ERR_SQLITE_ERROR, except one or two which we
> decided
> > > to
> > > be important so we get them their own svn error numbers.
> > >
> > > And it's not hiding any codes, they are still in the chain as
> > > err->child->apr_err (or svn_error_find_cause() if you prefer that).
> >
> > Users don't care about the enum mapping. All they see is an error like:
> >
> > svn: E230003: Unable to connect to a repository at URL
> > 'https://svn.apache.org:80'
> > svn: E230003: Error running context: An error occurred during SSL
> > communication
> > svn: E120171: APR does not understand this error code
> >
> > ^^^ I don't think this new line you added in r1501049 doesn't help any
user.
> >
> 
> Yes, that's a bug.  I'm not sure why you went to the effort of vetoing
> the patch and yelling at me in order to point it out.
> 
> This patch:
> 
> [[[
> Index: subversion/libsvn_ra_serf/util_error.c
> ==========================================================
> =========
> --- subversion/libsvn_ra_serf/util_error.c      (revision 1501266)
> +++ subversion/libsvn_ra_serf/util_error.c      (working copy)
> @@ -61,7 +61,6 @@ svn_ra_serf__wrap_err(apr_status_t status,
> 
>        if (serf_err_msg)
>          {
> -          err = svn_error_create(SVN_ERR_RA_SERF_WRAPPED_ERROR, err,
> NULL);
>            err_msg = serf_err_msg;
>          }
>        else
> @@ -96,6 +95,10 @@ svn_ra_serf__wrap_err(apr_status_t status,
>            err->message = msg;
>          }
>      }
> +
> +  /* Make the outer-most error code be a Subversion/APR one. */
> +  if (serf_err_msg)
> +    err = svn_error_create(SVN_ERR_RA_SERF_WRAPPED_ERROR, err,
> NULL);

I still don't see any good reason to do this.

SVN_ERR_RA_SERF_WRAPPED_ERROR didn't exist in Subversion 1.0, and this makes
it just as worse as the original error. An 1.0 binding user wouldn't be able
to use this error for the same reasoning.

We can't declare the only valid errors to be the errors in svn_error_codes.h
and we never will declare that to be the only valid error codes.

Every component can declare new integer values within its own ranges and
Subversion should be transparent with error codes: return them up the chain.

If 'svn' doesn't like this, that should be fixed in 'svn'.

Api users like those explicit error codes... and looking up the chain is not
without errors; loses information etc.

	Bert