You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by kf...@collab.net on 2006/02/01 13:03:47 UTC
Merging r18254 (BDB macro fix) into 1.3.x.
I was just about to merge r18254 from trunk into the 1.3.x branch,
when a closer investigation raised some questions.
r18254 is a parenthetization fix to the SVN_BDB_VERSION_AT_LEAST
macro. That macro was first introduced in r17484, whose summary is
"Working on issue #2449. Begin refactoring the BDB FS back-end in
preparation for implementing a cache of open BDB environment handles."
Now, r17484 was committed on trunk after 1.3.x branched, and has never
been merged into 1.3.x, according to the logs. It is also not listed
as a proposed backport in 1.3.x/STATUS. That seems odd -- I would
think 1.3.x would need it, presumably in the group of proposed
backports related to issue #2449 (the group beginning with r18144).
r18254 was approved for backport by four people, despite the fact that
the macro which it fixes does not even exist in 1.3.x without r17484.
This is not a criticism of those four people, I'm just trying to
understand what's going on here. Out of curiosity, I merged r18144
and friends into my 1.3.x working copy just to see what would happen,
and everything applied without conflict. That was very surprising!
Just looking over the log message of r17484, it's hard to believe that
change isn't a prerequisite for all of the later BDB work.
What am I missing here?
Below is the r18254 backport I would have committed to 1.3.x if I were
confident it was appropriate (yes, it builds without error):
[[[
Merge r18254 from trunk.
This has the effect of causing the SVN_BDB_VERSION_AT_LEAST macro to
exist in 1.3.x, although on trunk that macro already existed (since
r17484) and r18254 merely fixed a parenthetisation problem in it.
Approved by: brane, lundblad, malcolm, dlr
]]]
Index: STATUS
===================================================================
--- STATUS (revision 18313)
+++ STATUS (working copy)
@@ -72,12 +72,4 @@
Approved changes:
- * r18254
- Fix parenthetisation in the SVN_BDB_VERSION_AT_LEAST macro.
- Justification:
- Avoids precedence bugs when used in boolean expressions.
- Note:
- r18144 et al. depend on this.
- Votes:
- +1: brane, lundblad, malcolm, dlr
-
+ None pending.
Index: subversion/libsvn_fs_base/bdb/bdb_compat.h
===================================================================
--- subversion/libsvn_fs_base/bdb/bdb_compat.h (revision 18313)
+++ subversion/libsvn_fs_base/bdb/bdb_compat.h (working copy)
@@ -1,7 +1,7 @@
/* svn_bdb_compat.h --- Compatibility wrapper for different BDB versions.
*
* ====================================================================
- * Copyright (c) 2000-2004 CollabNet. All rights reserved.
+ * Copyright (c) 2000-2006 CollabNet. All rights reserved.
*
* This software is licensed as described in the file COPYING, which
* you should have received as part of this distribution. The terms
@@ -59,6 +59,11 @@
#define SVN_BDB_AUTO_RECOVER (0)
#endif
+/* Explicit BDB version check. */
+#define SVN_BDB_VERSION_AT_LEAST(major,minor) \
+ (DB_VERSION_MAJOR > (major) \
+ || (DB_VERSION_MAJOR == (major) && DB_VERSION_MINOR >= (minor)))
+
/* Parameter lists */
/* In BDB 4.1, DB->open takes a transaction parameter. We'll ignore it
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Merging r18254 (BDB macro fix) into 1.3.x.
Posted by Branko Čibej <br...@xbc.nu>.
kfogel@collab.net wrote:
> Branko Čibej <br...@xbc.nu> writes:
>
>> r17484 was committed on the bdb-fixes branch, not on trunk. r18144 is
>> the merge from that branch to trunk, so it includes this change.
>>
>
> Next time I should look at 'log -v' output. Sorry about that.
>
>
>> When I originally proposed r18254 for 1.3.1, I got the language about
>> backport dependencies backwards (as maxb correctly pointed out on IRC
>> last night).
>>
>>
>>> Below is the r18254 backport I would have committed to 1.3.x if I were
>>> confident it was appropriate (yes, it builds without error):
>>>
>>>
>> Yes, this patch is correct. Committing this to 1.3.x first will cause
>> a minor conflict when r18144 and friends get merged, but it has an
>> obvious resolution.
>>
>> I'd rather wait with this until the BDB fixes are merged, though,
>> because this macro won't be used otherwise.
>>
>
> Well, the BDB fixes name r18254 as a prereq, so I thought I'd merge
> that first (since it's already approved), *then* merge the rest of the
> changes and review/vote.
>
> Would you rather I didn't do things in that order?
>
I did say I got the dependencies inside-out when I wrote the STATUS
entries. :)
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Merging r18254 (BDB macro fix) into 1.3.x.
Posted by kf...@collab.net.
Branko Čibej <br...@xbc.nu> writes:
> r17484 was committed on the bdb-fixes branch, not on trunk. r18144 is
> the merge from that branch to trunk, so it includes this change.
Next time I should look at 'log -v' output. Sorry about that.
> When I originally proposed r18254 for 1.3.1, I got the language about
> backport dependencies backwards (as maxb correctly pointed out on IRC
> last night).
>
> > Below is the r18254 backport I would have committed to 1.3.x if I were
> > confident it was appropriate (yes, it builds without error):
> >
> Yes, this patch is correct. Committing this to 1.3.x first will cause
> a minor conflict when r18144 and friends get merged, but it has an
> obvious resolution.
>
> I'd rather wait with this until the BDB fixes are merged, though,
> because this macro won't be used otherwise.
Well, the BDB fixes name r18254 as a prereq, so I thought I'd merge
that first (since it's already approved), *then* merge the rest of the
changes and review/vote.
Would you rather I didn't do things in that order?
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Merging r18254 (BDB macro fix) into 1.3.x.
Posted by Julian Foad <ju...@btopenworld.com>.
Branko Čibej wrote:
>
> Ach, weel, now that you mention it, STATUS was complete when I first
> wrote the BDB entry, but later on Philip found a free-memory-read bug,
> and Ben found an incompatibility with pre-4.2 BDB.
[...]
> So it would be a good idea if I updated STATUS accordingly, yes?
Yes!
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Merging r18254 (BDB macro fix) into 1.3.x.
Posted by Branko Čibej <br...@xbc.nu>.
kfogel@collab.net wrote:
> Branko Čibej <br...@xbc.nu> writes:
>
>> Philip Martin wrote:
>>
>>> Branko Čibej <br...@xbc.nu> writes:
>>>
>>>
>>>> I'd rather wait with this until the BDB fixes are merged,
>>>>
>>> What about the get_flags patch you mentioned a few days ago, don't we
>>> need that as well?
>>>
>> Yes, we do. It's in my tree.
>>
>
> Er, let me just get straight to the point :-)...
>
> Does what's in 1.3.x/STATUS now reflect everything one would need to
> review and vote on these changes? If not, can it? (Or if they're not
> ready for that yet, can STATUS indicate that?)
>
Ach, weel, now that you mention it, STATUS was complete when I first
wrote the BDB entry, but later on Philip found a free-memory-read bug,
and Ben found an incompatibility with pre-4.2 BDB.
I'm working on the first issue (and cursing APR, pools, pool cleanups,
life, and the universe in general), and I've got the second fixed in my
tree.
So it would be a good idea if I updated STATUS accordingly, yes?
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Merging r18254 (BDB macro fix) into 1.3.x.
Posted by kf...@collab.net.
Branko Čibej <br...@xbc.nu> writes:
> Philip Martin wrote:
> > Branko Čibej <br...@xbc.nu> writes:
> >
> >> I'd rather wait with this until the BDB fixes are merged,
> >
> > What about the get_flags patch you mentioned a few days ago, don't we
> > need that as well?
>
> Yes, we do. It's in my tree.
Er, let me just get straight to the point :-)...
Does what's in 1.3.x/STATUS now reflect everything one would need to
review and vote on these changes? If not, can it? (Or if they're not
ready for that yet, can STATUS indicate that?)
Thanks,
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Merging r18254 (BDB macro fix) into 1.3.x.
Posted by Branko Čibej <br...@xbc.nu>.
Philip Martin wrote:
> Branko Čibej <br...@xbc.nu> writes:
>
>
>> I'd rather wait with this until the BDB fixes are merged,
>>
>
> What about the get_flags patch you mentioned a few days ago, don't we
> need that as well?
>
Yes, we do. It's in my tree.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Merging r18254 (BDB macro fix) into 1.3.x.
Posted by Philip Martin <ph...@codematters.co.uk>.
Branko Čibej <br...@xbc.nu> writes:
> I'd rather wait with this until the BDB fixes are merged,
What about the get_flags patch you mentioned a few days ago, don't we
need that as well?
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Merging r18254 (BDB macro fix) into 1.3.x.
Posted by Branko Čibej <br...@xbc.nu>.
kfogel@collab.net wrote:
> What am I missing here?
>
r17484 was committed on the bdb-fixes branch, not on trunk. r18144 is
the merge from that branch to trunk, so it includes this change.
When I originally proposed r18254 for 1.3.1, I got the language about
backport dependencies backwards (as maxb correctly pointed out on IRC
last night).
> Below is the r18254 backport I would have committed to 1.3.x if I were
> confident it was appropriate (yes, it builds without error):
>
Yes, this patch is correct. Committing this to 1.3.x first will cause a
minor conflict when r18144 and friends get merged, but it has an obvious
resolution.
I'd rather wait with this until the BDB fixes are merged, though,
because this macro won't be used otherwise.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org