You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by wr...@apache.org on 2005/03/17 03:07:52 UTC
svn commit: r157852 - in apr/apr-iconv/trunk: CHANGES lib/api_version.c
lib/iconv_module.c
Author: wrowe
Date: Wed Mar 16 18:07:51 2005
New Revision: 157852
URL: http://svn.apache.org/viewcvs?view=rev&rev=157852
Log:
Update iconv_module to search APR_ICONV1_PATH, ensuring we don't
collide with apr-iconv version 0.9.x flavor iconv modules
(which weren't compatible.) This was particularly an issue when
mixing flavors of subversion, apache and other apr-based apps
when both 0.9 and 1.x are required.
Reported by: Curt Arnold <carnold apache.org>
Modified:
apr/apr-iconv/trunk/CHANGES
apr/apr-iconv/trunk/lib/api_version.c
apr/apr-iconv/trunk/lib/iconv_module.c
Modified: apr/apr-iconv/trunk/CHANGES
URL: http://svn.apache.org/viewcvs/apr/apr-iconv/trunk/CHANGES?view=diff&r1=157851&r2=157852
==============================================================================
--- apr/apr-iconv/trunk/CHANGES (original)
+++ apr/apr-iconv/trunk/CHANGES Wed Mar 16 18:07:51 2005
@@ -1,5 +1,10 @@
Changes with APR-ICONV 1.1
+ *) APR_ICONV1_PATH distinguishes between our apr-iconv 0.9 and 1.x
+ flavors (changed pool arguments to several internal functions, so the
+ loadable charset modules are not binary compatible.) The older 0.9
+ flavor APR_ICONV_PATH won't be searched. [William Rowe]
+
Changes with APR-ICONV 1.0.2
*) Fix libapriconv.rc for Win32 builds [William Rowe, Justin Erenkrantz]
Modified: apr/apr-iconv/trunk/lib/api_version.c
URL: http://svn.apache.org/viewcvs/apr/apr-iconv/trunk/lib/api_version.c?view=diff&r1=157851&r2=157852
==============================================================================
--- apr/apr-iconv/trunk/lib/api_version.c (original)
+++ apr/apr-iconv/trunk/lib/api_version.c Wed Mar 16 18:07:51 2005
@@ -13,8 +13,6 @@
* limitations under the License.
*/
-#include "apr_general.h" /* for APR_STRINGIFY */
-
#include "apr_iconv.h"
#include "api_version.h"
Modified: apr/apr-iconv/trunk/lib/iconv_module.c
URL: http://svn.apache.org/viewcvs/apr/apr-iconv/trunk/lib/iconv_module.c?view=diff&r1=157851&r2=157852
==============================================================================
--- apr/apr-iconv/trunk/lib/iconv_module.c (original)
+++ apr/apr-iconv/trunk/lib/iconv_module.c Wed Mar 16 18:07:51 2005
@@ -39,6 +39,7 @@
#include "apr_strings.h"
#include "apr_tables.h"
#include "apr_lib.h"
+#include "api_version.h"
#include <string.h>
#include <stdlib.h>
@@ -49,6 +50,8 @@
#include "charset_alias.h"
#endif
+#define APR_ICONV_PATH "APR_ICONV" API_STRINGIFY(API_MAJOR_VERSION) "_PATH"
+
static apr_status_t
iconv_getpathname(char *buffer, const char *dir, const char *name, apr_pool_t *ctx)
{
@@ -93,7 +96,7 @@
while (0 != (*ptr++ = apr_tolower(*name++)))
;
- if (!apr_env_get(&ptr, "APR_ICONV_PATH", subpool)
+ if (!apr_env_get(&ptr, APR_ICONV_PATH, subpool)
&& !apr_filepath_list_split(&pathelts, ptr, subpool))
{
int i;
Re: svn commit: r157852 - in apr/apr-iconv/trunk: CHANGES
lib/api_version.c lib/iconv_module.c
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 08:16 PM 3/16/2005, Sander Striker wrote:
>wrowe@apache.org wrote:
>>Author: wrowe
>>Date: Wed Mar 16 18:07:51 2005
>>New Revision: 157852
>>URL: http://svn.apache.org/viewcvs?view=rev&rev=157852
>
>[...]
>>Modified: apr/apr-iconv/trunk/lib/iconv_module.c
>>URL: http://svn.apache.org/viewcvs/apr/apr-iconv/trunk/lib/iconv_module.c?view=diff&r1=157851&r2=157852
>>==============================================================================
>>--- apr/apr-iconv/trunk/lib/iconv_module.c (original)
>>+++ apr/apr-iconv/trunk/lib/iconv_module.c Wed Mar 16 18:07:51 2005
>
>[...]
>>@@ -49,6 +50,8 @@
>> #include "charset_alias.h"
>> #endif
>>
>>+#define APR_ICONV_PATH "APR_ICONV" API_STRINGIFY(API_MAJOR_VERSION) "_PATH"
>
>API_STRINGIFY? When did we grow that? Is this materially different than APR_STRINGIFY?
No it's identical, it's also trivial and doesn't require some
'universal declaration.'
Re: Hold Noses (was Re: svn commit: r157852 - in
apr/apr-iconv/trunk: CHANGES lib/api_version.c lib/iconv_module.c)
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Thank you Sander!!!
Although yes, I would wish that viewcvs would do so, I understand how
differently deltas are stacked in svn vs cvs. Obviously, for a given
given pair of mechanisms, each of the functions will perform differently
in terms of speed/bandwidth/storage to one another. I'll spend my own
CPUs cycles gladly, to have this feature back.
And also obviously, this particular change wasn't hard to backtrack,
but your thorough example of praise/blame below has set me on the right
track to hunt those down. Bless you :)
p.p.s. I've been frequently to red-bean and own hardcopy, but does
anyone recommend a well written "svn for cvs users" quick reference,
both modestly terse and complete?
At 03:24 AM 3/17/2005, Sander Striker wrote:
>Annotation is currently disabled in viewcvs. It does work, but there
>are some performance issues with it. There are other ways to get to
>the information though.
>
>Let me show you:
>
> svn praise http://svn.apache.org/repos/asf/apr/apr-iconv/trunk/include/api_version.h
>
> ...
> 157664 jerenkrantz #ifndef API_STRINGIFY
> 157664 jerenkrantz /** Properly quote a value as a string in the C preprocessor */
> 157664 jerenkrantz #define API_STRINGIFY(n) API_STRINGIFY_HELPER(n)
> 157664 jerenkrantz /** Helper macro for API_STRINGIFY */
> 157664 jerenkrantz #define API_STRINGIFY_HELPER(n) #n
> 157664 jerenkrantz #endif
> ...
>
>Ah, rev 157664. What was that commit?
>
> svn log -r 157664 -v
>
> ------------------------------------------------------------------------
> r157664 | jerenkrantz | 2005-03-16 05:25:48 +0100 (Wed, 16 Mar 2005) | 3 lines
> Changed paths:
> M /apr/apr-iconv/trunk/CHANGES
> M /apr/apr-iconv/trunk/include/api_version.h
> M /apr/apr-iconv/trunk/libapriconv.dsp
> A /apr/apr-iconv/trunk/libapriconv.rc
>
> Fix libapriconv.rc for Win32 release builds.
> Also sync up CHANGES and bump trunk to 1.1.0-dev.
>
> ------------------------------------------------------------------------
>
>Hmm, let's look at the CHANGES entry before we entirely praise jerenkrantz.
>
> svn diff -r 157663:157664 http://svn.apache.org/repos/asf/apr/apr-iconv/trunk/CHANGES
>
> Index: CHANGES
> ===================================================================
> --- CHANGES (revision 157663)
> +++ CHANGES (revision 157664)
> @@ -1,5 +1,13 @@
> -Changes with APR-ICONV 1.0
> +Changes with APR-ICONV 1.1
> +Changes with APR-ICONV 1.0.2
> +
> + *) Fix libapriconv.rc for Win32 builds [William Rowe, Justin Erenkrantz]
> +
> +Changes with APR-ICONV 1.0.1
> +
> +Changes with APR-ICONV 1.0.0
> +
> *) Add the possiblity of a DESTDIR prefix to Makefile.in to make it
> consistent with the behaviour of apr and apr-util. [Graham Leggett]
>
>
>So, this must have had some discussion on list somewhere before the commit.
>The log said it was committed 16th of march, so lets have a look
>
> http://mail-archives.apache.org/mod_mbox/apr-dev/
>
>[...stops looking for the mystery patch and starts packing...]
Re: Hold Noses (was Re: svn commit: r157852 - in apr/apr-iconv/trunk:
CHANGES lib/api_version.c lib/iconv_module.c)
Posted by Sander Striker <st...@apache.org>.
William A. Rowe, Jr. wrote:
> At 09:57 PM 3/16/2005, Justin Erenkrantz wrote:
>
>>On Thu, Mar 17, 2005 at 03:16:44AM +0100, Sander Striker wrote:
>>
>>>API_STRINGIFY? When did we grow that? Is this materially different than
>>>APR_STRINGIFY?
>>
>>I know, I know. OtherBill's patches required us to remove any dependencies
>>upon APR_* functions because the Win32 RC compiler can't handle it. I held my
>>nose during the commit... -- justin
>
> functions. That's an include. Wasn't required, simply trivial.
>
> However it was rather pointless to include apr for a two line #define,
> don't you think? Apparently not.
Duping is always questionable. We can simply add a comment like:
/* Duping the APR_STRINGIFY macros here, because the Win32 RC compiler
* doesn't like includes in the way we set things up.
*/
End of story.
> If you have an issue, rather than grabbing a tissue, try addressing
> it to the list at that time, as opposed to personal snipes.
I assume everyone lets minor issues slide once in a while, weighing the
energy you have to spend on it against the desired result.
> Once again you sent me hunting for an [annotated] viewsvn history to
> track down which patch I added that in.
Noone is sending you hunting, that is something you decide to do on
your own.
> Oh wait - svn can't do that. Score another for premature adoption.
Ok, here it would have been best for you to grab a tissue.
Have you even tried running svn blame (or if you don't like that name:
svn praise)? Or if you're a TortoiseSVN user tried running Blame from
the context menu?
Annotation is currently disabled in viewcvs. It does work, but there
are some performance issues with it. There are other ways to get to
the information though.
Let me show you:
svn praise http://svn.apache.org/repos/asf/apr/apr-iconv/trunk/include/api_version.h
...
157664 jerenkrantz #ifndef API_STRINGIFY
157664 jerenkrantz /** Properly quote a value as a string in the C preprocessor */
157664 jerenkrantz #define API_STRINGIFY(n) API_STRINGIFY_HELPER(n)
157664 jerenkrantz /** Helper macro for API_STRINGIFY */
157664 jerenkrantz #define API_STRINGIFY_HELPER(n) #n
157664 jerenkrantz #endif
...
Ah, rev 157664. What was that commit?
svn log -r 157664 -v
------------------------------------------------------------------------
r157664 | jerenkrantz | 2005-03-16 05:25:48 +0100 (Wed, 16 Mar 2005) | 3 lines
Changed paths:
M /apr/apr-iconv/trunk/CHANGES
M /apr/apr-iconv/trunk/include/api_version.h
M /apr/apr-iconv/trunk/libapriconv.dsp
A /apr/apr-iconv/trunk/libapriconv.rc
Fix libapriconv.rc for Win32 release builds.
Also sync up CHANGES and bump trunk to 1.1.0-dev.
------------------------------------------------------------------------
Hmm, let's look at the CHANGES entry before we entirely praise jerenkrantz.
svn diff -r 157663:157664 http://svn.apache.org/repos/asf/apr/apr-iconv/trunk/CHANGES
Index: CHANGES
===================================================================
--- CHANGES (revision 157663)
+++ CHANGES (revision 157664)
@@ -1,5 +1,13 @@
-Changes with APR-ICONV 1.0
+Changes with APR-ICONV 1.1
+Changes with APR-ICONV 1.0.2
+
+ *) Fix libapriconv.rc for Win32 builds [William Rowe, Justin Erenkrantz]
+
+Changes with APR-ICONV 1.0.1
+
+Changes with APR-ICONV 1.0.0
+
*) Add the possiblity of a DESTDIR prefix to Makefile.in to make it
consistent with the behaviour of apr and apr-util. [Graham Leggett]
So, this must have had some discussion on list somewhere before the commit.
The log said it was committed 16th of march, so lets have a look
http://mail-archives.apache.org/mod_mbox/apr-dev/
[...stops looking for the mystery patch and starts packing...]
Sander
Hold Noses (was Re: svn commit: r157852 - in
apr/apr-iconv/trunk: CHANGES lib/api_version.c lib/iconv_module.c)
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 09:57 PM 3/16/2005, Justin Erenkrantz wrote:
>On Thu, Mar 17, 2005 at 03:16:44AM +0100, Sander Striker wrote:
>> API_STRINGIFY? When did we grow that? Is this materially different than
>> APR_STRINGIFY?
>
>I know, I know. OtherBill's patches required us to remove any dependencies
>upon APR_* functions because the Win32 RC compiler can't handle it. I held my
>nose during the commit... -- justin
functions. That's an include. Wasn't required, simply trivial.
However it was rather pointless to include apr for a two line #define,
don't you think? Apparently not.
If you have an issue, rather than grabbing a tissue, try addressing
it to the list at that time, as opposed to personal snipes.
Once again you sent me hunting for an [annotated] viewsvn history to
track down which patch I added that in. Oh wait - svn can't do that.
Score another for premature adoption.
Re: svn commit: r157852 - in apr/apr-iconv/trunk: CHANGES lib/api_version.c lib/iconv_module.c
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Thu, Mar 17, 2005 at 03:16:44AM +0100, Sander Striker wrote:
> API_STRINGIFY? When did we grow that? Is this materially different than
> APR_STRINGIFY?
I know, I know. OtherBill's patches required us to remove any dependencies
upon APR_* functions because the Win32 RC compiler can't handle it. I held my
nose during the commit... -- justin
Re: svn commit: r157852 - in apr/apr-iconv/trunk: CHANGES lib/api_version.c
lib/iconv_module.c
Posted by Sander Striker <st...@apache.org>.
wrowe@apache.org wrote:
> Author: wrowe
> Date: Wed Mar 16 18:07:51 2005
> New Revision: 157852
>
> URL: http://svn.apache.org/viewcvs?view=rev&rev=157852
[...]
> Modified: apr/apr-iconv/trunk/lib/iconv_module.c
> URL: http://svn.apache.org/viewcvs/apr/apr-iconv/trunk/lib/iconv_module.c?view=diff&r1=157851&r2=157852
> ==============================================================================
> --- apr/apr-iconv/trunk/lib/iconv_module.c (original)
> +++ apr/apr-iconv/trunk/lib/iconv_module.c Wed Mar 16 18:07:51 2005
[...]
> @@ -49,6 +50,8 @@
> #include "charset_alias.h"
> #endif
>
> +#define APR_ICONV_PATH "APR_ICONV" API_STRINGIFY(API_MAJOR_VERSION) "_PATH"
API_STRINGIFY? When did we grow that? Is this materially different than APR_STRINGIFY?
Sander