You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by mi...@apache.org on 2012/04/01 17:23:45 UTC
svn commit: r1308135 - in /apr/apr-util/branches/1.4.x: ./ CHANGES
crypto/apr_crypto.c
Author: minfrin
Date: Sun Apr 1 15:23:45 2012
New Revision: 1308135
URL: http://svn.apache.org/viewvc?rev=1308135&view=rev
Log:
Backport:
apr_crypto: Ensure the *driver variable is initialised when a statically
compiled library is initialised for the first time.
Modified:
apr/apr-util/branches/1.4.x/ (props changed)
apr/apr-util/branches/1.4.x/CHANGES
apr/apr-util/branches/1.4.x/crypto/apr_crypto.c
Propchange: apr/apr-util/branches/1.4.x/
------------------------------------------------------------------------------
Merged /apr/apr/trunk:r1308131
Modified: apr/apr-util/branches/1.4.x/CHANGES
URL: http://svn.apache.org/viewvc/apr/apr-util/branches/1.4.x/CHANGES?rev=1308135&r1=1308134&r2=1308135&view=diff
==============================================================================
--- apr/apr-util/branches/1.4.x/CHANGES [utf-8] (original)
+++ apr/apr-util/branches/1.4.x/CHANGES [utf-8] Sun Apr 1 15:23:45 2012
@@ -1,6 +1,9 @@
-*- coding: utf-8 -*-
Changes with APR-util 1.4.3
+ *) apr_crypto: Ensure the *driver variable is initialised when a statically
+ compiled library is initialised for the first time. [Graham Leggett]
+
*) apr_crypto: Ensure the *driver variable is initialised when the library
has already been loaded. Fix ported from apr_dbd. [Graham Leggett]
Modified: apr/apr-util/branches/1.4.x/crypto/apr_crypto.c
URL: http://svn.apache.org/viewvc/apr/apr-util/branches/1.4.x/crypto/apr_crypto.c?rev=1308135&r1=1308134&r2=1308135&view=diff
==============================================================================
--- apr/apr-util/branches/1.4.x/crypto/apr_crypto.c (original)
+++ apr/apr-util/branches/1.4.x/crypto/apr_crypto.c Sun Apr 1 15:23:45 2012
@@ -68,13 +68,14 @@ typedef struct apr_crypto_clear_t {
} apr_crypto_clear_t;
#if !APU_DSO_BUILD
-#define DRIVER_LOAD(name,driver,pool,params,rv,result) \
+#define DRIVER_LOAD(name,driver_name,pool,params,rv,result) \
{ \
- extern const apr_crypto_driver_t driver; \
- apr_hash_set(drivers,name,APR_HASH_KEY_STRING,&driver); \
- if (driver.init) { \
- rv = driver.init(pool, params, result); \
+ extern const apr_crypto_driver_t driver_name; \
+ apr_hash_set(drivers,name,APR_HASH_KEY_STRING,&driver_name); \
+ if (driver_name.init) { \
+ rv = driver_name.init(pool, params, result); \
} \
+ *driver = &driver_name; \
}
#endif
Re: svn commit: r1308135 - in /apr/apr-util/branches/1.4.x: ./ CHANGES crypto/apr_crypto.c
Posted by Graham Leggett <mi...@sharp.fm>.
On 01 Apr 2012, at 10:55 PM, Greg Stein wrote:
> -131 has no edit to CHANGES.
>
> When performing backports, you should commit *just* the backport. If
> you want to make other changes, then keep them in a separate commit.
>
> Combining changes like this is poor branch policy. It means that we
> can no longer trust that a backport is *JUST* the backport. We have to
> investigate to determine whether other changes were made in the
> commit.
I personally agree with you, but I lost that fight when the issue of CHANGES files was thrashed out on this list.
I'm sure those who set the current CHANGES file policy will step forward at this point and explain their reasoning behind it.
Regards,
Graham
--
Re: svn commit: r1308135 - in /apr/apr-util/branches/1.4.x: ./ CHANGES crypto/apr_crypto.c
Posted by Graham Leggett <mi...@sharp.fm>.
On 01 Apr 2012, at 11:29 PM, Greg Stein wrote:
>> What about svn:mergeinfo?
>
> So? Do I need to run 'svn diff' on every commit to see what happened?
Isn't that the very definition of svn diff? To tell us what happened?
Personally, I'm am utterly exhausted by constantly being told by this person and then that person that some or other completely undocumented pattern isn't being followed. All I want to do is fix some bugs, get a release out the door, and help some long suffering Windows folks who are having problems with static builds.
Between you and Jeff and whoever else wants to chime in, formulate a proper *workable* backport strategy. *document* it properly so that it can be found easily at http://apr.apache.org, and I will happily follow whatever you've decided to do.
Regards,
Graham
--
Re: svn commit: r1308135 - in /apr/apr-util/branches/1.4.x: ./
CHANGES crypto/apr_crypto.c
Posted by Greg Stein <gs...@gmail.com>.
On Sun, Apr 1, 2012 at 17:22, Graham Leggett <mi...@sharp.fm> wrote:
> On 01 Apr 2012, at 11:13 PM, Greg Stein wrote:
>
>> These two backports today were reckless. Both of them were not
>> "touchless" to use your term. And worse: that fact was not documented
>> in the log message. "Backport" is all it said. Not what revision was
>> backported. Not that other changes were made beyond that
>> (unidentified) revision.
>
> What about svn:mergeinfo?
So? Do I need to run 'svn diff' on every commit to see what happened?
If you're arguing that, then don't add a log message. Leave it blank.
Make people run diff on every change to see what happened.
The simple fact is that branches need to be held to a higher standard
than trunk. We don't do random development on branches. We simply
backport changes. The log message should note WHAT was backported. (I
have edited the log message for today's two backports, so they now
reflect what happened in those commits)
svn:mergeinfo records the backports, but it is hardly as informative
as the log message. Especially when other changes are blended in.
-g
Re: svn commit: r1308135 - in /apr/apr-util/branches/1.4.x: ./ CHANGES crypto/apr_crypto.c
Posted by Graham Leggett <mi...@sharp.fm>.
On 01 Apr 2012, at 11:13 PM, Greg Stein wrote:
> These two backports today were reckless. Both of them were not
> "touchless" to use your term. And worse: that fact was not documented
> in the log message. "Backport" is all it said. Not what revision was
> backported. Not that other changes were made beyond that
> (unidentified) revision.
What about svn:mergeinfo?
Little-Net:apr-util-1.4 minfrin$ svn diff -c 1308135
[snip]
Property changes on: .
___________________________________________________________________
Modified: svn:mergeinfo
Merged /apr/apr/trunk:r1308131
My understanding was that svn:mergeinfo is used to track exactly these kinds of changes, it not please say so.
Regards,
Graham
--
Re: svn commit: r1308135 - in /apr/apr-util/branches/1.4.x: ./ CHANGES
crypto/apr_crypto.c
Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 4/1/2012 4:29 PM, Jeff Trawick wrote:
>
> I agree that a log message is busted if it doesn't mention the trunk
> revision(s) for the code. (I don't know how the mergeinfo shows up
> when looking at the revisions later.)
CVS/Template
... ah, to enjoy simpler days again.
Re: svn commit: r1308135 - in /apr/apr-util/branches/1.4.x: ./
CHANGES crypto/apr_crypto.c
Posted by Jeff Trawick <tr...@gmail.com>.
On Sun, Apr 1, 2012 at 5:13 PM, Greg Stein <gs...@gmail.com> wrote:
> On Sun, Apr 1, 2012 at 17:04, Jeff Trawick <tr...@gmail.com> wrote:
>> On Sun, Apr 1, 2012 at 4:55 PM, Greg Stein <gs...@gmail.com> wrote:
>>> On Sun, Apr 1, 2012 at 11:23, <mi...@apache.org> wrote:
>>>> Author: minfrin
>>>> Date: Sun Apr 1 15:23:45 2012
>>>> New Revision: 1308135
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1308135&view=rev
>>>> Log:
>>>> Backport:
>>>> apr_crypto: Ensure the *driver variable is initialised when a statically
>>>> compiled library is initialised for the first time.
>>>>
>>>> Modified:
>>>> apr/apr-util/branches/1.4.x/ (props changed)
>>>> apr/apr-util/branches/1.4.x/CHANGES
>>>> apr/apr-util/branches/1.4.x/crypto/apr_crypto.c
>>>>
>>>> Propchange: apr/apr-util/branches/1.4.x/
>>>> ------------------------------------------------------------------------------
>>>> Merged /apr/apr/trunk:r1308131
>>>
>>> -131 has no edit to CHANGES.
>>>
>>> When performing backports, you should commit *just* the backport. If
>>> you want to make other changes, then keep them in a separate commit.
>>>
>>> Combining changes like this is poor branch policy. It means that we
>>> can no longer trust that a backport is *JUST* the backport. We have to
>>> investigate to determine whether other changes were made in the
>>> commit.
>>
>> That adds an extra step to looking at the code changes for a CHANGES
>> entry, if indeed the merger/backporter remembered to list the code
>> change in the CHANGES commit. (Otherwise it is more painful.)
>
> I'm not sure that I follow that statement.
>
> My point is: if you do a backport of a commit, then do *just* the
> backport. Throwing other work into that commit leads to insanity.
I agree that throwing in unrelated modifications leads to insanity.
(But our definition of unrelated probably differs; fixing CHANGES in
the commit has not led to insanity so far and has avoided the
potential that the subsequent CHANGES commit didn't reference the code
change.)
>> I guess you mean that the "trust" part comes not from backports in
>> general but only from a a "touchless" merge. Generally a "backport"
>> may require minor changes to reflect other differences in the branch.
>
> For that kind of problem, in Subversion, we create a
> branch-of-the-branch then backport the revision, then make the
> necessary edits to make the backport conform to the branch. Thus, when
> we touch branches/1.N.x, we are doing a simple backport merge. Again:
> no other changes in that revision beyond JUST the backport.
>
> So yes: I'm saying touchless merges are the proper way to manage branches.
Then see what kind of buy-in you get for that in a new thread.
> These two backports today were reckless. Both of them were not
> "touchless" to use your term. And worse: that fact was not documented
> in the log message. "Backport" is all it said. Not what revision was
> backported. Not that other changes were made beyond that
> (unidentified) revision.
I agree that a log message is busted if it doesn't mention the trunk
revision(s) for the code. (I don't know how the mergeinfo shows up
when looking at the revisions later.)
> I can maybe understand a policy that says CHANGES should be edited
> directly on the branch (since, at this point, trunk CHANGES and branch
> CHANGES are completel dissimilar). But those edits should not be part
> of backport merges.
shrug
Re: svn commit: r1308135 - in /apr/apr-util/branches/1.4.x: ./
CHANGES crypto/apr_crypto.c
Posted by Greg Stein <gs...@gmail.com>.
On Sun, Apr 1, 2012 at 17:04, Jeff Trawick <tr...@gmail.com> wrote:
> On Sun, Apr 1, 2012 at 4:55 PM, Greg Stein <gs...@gmail.com> wrote:
>> On Sun, Apr 1, 2012 at 11:23, <mi...@apache.org> wrote:
>>> Author: minfrin
>>> Date: Sun Apr 1 15:23:45 2012
>>> New Revision: 1308135
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1308135&view=rev
>>> Log:
>>> Backport:
>>> apr_crypto: Ensure the *driver variable is initialised when a statically
>>> compiled library is initialised for the first time.
>>>
>>> Modified:
>>> apr/apr-util/branches/1.4.x/ (props changed)
>>> apr/apr-util/branches/1.4.x/CHANGES
>>> apr/apr-util/branches/1.4.x/crypto/apr_crypto.c
>>>
>>> Propchange: apr/apr-util/branches/1.4.x/
>>> ------------------------------------------------------------------------------
>>> Merged /apr/apr/trunk:r1308131
>>
>> -131 has no edit to CHANGES.
>>
>> When performing backports, you should commit *just* the backport. If
>> you want to make other changes, then keep them in a separate commit.
>>
>> Combining changes like this is poor branch policy. It means that we
>> can no longer trust that a backport is *JUST* the backport. We have to
>> investigate to determine whether other changes were made in the
>> commit.
>
> That adds an extra step to looking at the code changes for a CHANGES
> entry, if indeed the merger/backporter remembered to list the code
> change in the CHANGES commit. (Otherwise it is more painful.)
I'm not sure that I follow that statement.
My point is: if you do a backport of a commit, then do *just* the
backport. Throwing other work into that commit leads to insanity.
> I guess you mean that the "trust" part comes not from backports in
> general but only from a a "touchless" merge. Generally a "backport"
> may require minor changes to reflect other differences in the branch.
For that kind of problem, in Subversion, we create a
branch-of-the-branch then backport the revision, then make the
necessary edits to make the backport conform to the branch. Thus, when
we touch branches/1.N.x, we are doing a simple backport merge. Again:
no other changes in that revision beyond JUST the backport.
So yes: I'm saying touchless merges are the proper way to manage branches.
These two backports today were reckless. Both of them were not
"touchless" to use your term. And worse: that fact was not documented
in the log message. "Backport" is all it said. Not what revision was
backported. Not that other changes were made beyond that
(unidentified) revision.
I can maybe understand a policy that says CHANGES should be edited
directly on the branch (since, at this point, trunk CHANGES and branch
CHANGES are completel dissimilar). But those edits should not be part
of backport merges.
-g
Re: svn commit: r1308135 - in /apr/apr-util/branches/1.4.x: ./
CHANGES crypto/apr_crypto.c
Posted by Jeff Trawick <tr...@gmail.com>.
On Sun, Apr 1, 2012 at 4:55 PM, Greg Stein <gs...@gmail.com> wrote:
> On Sun, Apr 1, 2012 at 11:23, <mi...@apache.org> wrote:
>> Author: minfrin
>> Date: Sun Apr 1 15:23:45 2012
>> New Revision: 1308135
>>
>> URL: http://svn.apache.org/viewvc?rev=1308135&view=rev
>> Log:
>> Backport:
>> apr_crypto: Ensure the *driver variable is initialised when a statically
>> compiled library is initialised for the first time.
>>
>> Modified:
>> apr/apr-util/branches/1.4.x/ (props changed)
>> apr/apr-util/branches/1.4.x/CHANGES
>> apr/apr-util/branches/1.4.x/crypto/apr_crypto.c
>>
>> Propchange: apr/apr-util/branches/1.4.x/
>> ------------------------------------------------------------------------------
>> Merged /apr/apr/trunk:r1308131
>
> -131 has no edit to CHANGES.
>
> When performing backports, you should commit *just* the backport. If
> you want to make other changes, then keep them in a separate commit.
>
> Combining changes like this is poor branch policy. It means that we
> can no longer trust that a backport is *JUST* the backport. We have to
> investigate to determine whether other changes were made in the
> commit.
That adds an extra step to looking at the code changes for a CHANGES
entry, if indeed the merger/backporter remembered to list the code
change in the CHANGES commit. (Otherwise it is more painful.)
I guess you mean that the "trust" part comes not from backports in
general but only from a a "touchless" merge. Generally a "backport"
may require minor changes to reflect other differences in the branch.
Re: svn commit: r1308135 - in /apr/apr-util/branches/1.4.x: ./
CHANGES crypto/apr_crypto.c
Posted by Greg Stein <gs...@gmail.com>.
On Sun, Apr 1, 2012 at 11:23, <mi...@apache.org> wrote:
> Author: minfrin
> Date: Sun Apr 1 15:23:45 2012
> New Revision: 1308135
>
> URL: http://svn.apache.org/viewvc?rev=1308135&view=rev
> Log:
> Backport:
> apr_crypto: Ensure the *driver variable is initialised when a statically
> compiled library is initialised for the first time.
>
> Modified:
> apr/apr-util/branches/1.4.x/ (props changed)
> apr/apr-util/branches/1.4.x/CHANGES
> apr/apr-util/branches/1.4.x/crypto/apr_crypto.c
>
> Propchange: apr/apr-util/branches/1.4.x/
> ------------------------------------------------------------------------------
> Merged /apr/apr/trunk:r1308131
-131 has no edit to CHANGES.
When performing backports, you should commit *just* the backport. If
you want to make other changes, then keep them in a separate commit.
Combining changes like this is poor branch policy. It means that we
can no longer trust that a backport is *JUST* the backport. We have to
investigate to determine whether other changes were made in the
commit.
-g