You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Joe Swatosh <jo...@gmail.com> on 2009/05/22 06:26:50 UTC
[Patch] Make Ruby and Perl shared libraries .dll again
Make the Ruby and Perl shared libraries .dll instead of .pyd. There is
probably a smarter way to do this with the ._extension_map, but I'm no
kind of Python expert.
[[[
Partially revert r37331 "On Windows, shared libs for Python bindings
should be *.pyd not *.dll." Change back to .dll for Ruby and Perl.
* build/generator/gen_base.py (TargetSWIG.add_dependencies): Mark Python
wrapper shared libs as type "pyd" not "lib".
]]]
===================================================================
--- build/generator/gen_base.py (revision 37787)
+++ build/generator/gen_base.py (working copy)
@@ -550,12 +550,13 @@
# Extract SWIG module name from .i file name
module_name = iname[:4] != 'svn_' and iname[:-2] or iname[4:-2]
- lib_extension = self.gen_obj._extension_map['pyd', 'target']
+ lib_extension = self.gen_obj._extension_map['lib', 'target']
if self.lang == "ruby":
lib_filename = module_name + lib_extension
elif self.lang == "perl":
lib_filename = '_' + module_name.capitalize() + lib_extension
else:
+ lib_extension = self.gen_obj._extension_map['pyd', 'target']
lib_filename = '_' + module_name + lib_extension
self.name = self.lang + '_' + module_name
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2352771
Re: [Patch] Make Ruby and Perl shared libraries .dll again
Posted by Joe Swatosh <jo...@gmail.com>.
On Fri, May 29, 2009 at 3:56 AM, Stefan Sperling <st...@elego.de> wrote:
> On Thu, May 28, 2009 at 10:23:54PM -0700, Joe Swatosh wrote:
>> This is not part of the Ruby bindings so it is not for me to commit
>> without approval by a full committer. Is the patch acceptable with the
>> log message amended below?
>>
>> --
>> Joe
>>
>> [[[
>>
>> Partially revert r37331 "On Windows, shared libs for Python bindings
>> should be *.pyd not *.dll." In addition to changing the shared libs for
>> the Python bindings, r37331 inadvertently changed the extensions of the
>> shared libraries produced for the Perl and Ruby bindings from .dll to
>> .pyd. Change the extensions back to .dll for the Ruby and Perl
>> bindings.
>>
>> * build/generator/gen_base.py (TargetSWIG.add_dependencies): Mark Python
>> wrapper shared libs as type "pyd" not "lib".
>>
>> ]]]
>>
>> ===================================================================
>> --- build/generator/gen_base.py (revision 37787)
>> +++ build/generator/gen_base.py (working copy)
>> @@ -550,12 +550,13 @@
>> # Extract SWIG module name from .i file name
>> module_name = iname[:4] != 'svn_' and iname[:-2] or iname[4:-2]
>>
>> - lib_extension = self.gen_obj._extension_map['pyd', 'target']
>> + lib_extension = self.gen_obj._extension_map['lib', 'target']
>> if self.lang == "ruby":
>> lib_filename = module_name + lib_extension
>> elif self.lang == "perl":
>> lib_filename = '_' + module_name.capitalize() + lib_extension
>> else:
>> + lib_extension = self.gen_obj._extension_map['pyd', 'target']
>
> Why put this in an else clause? This code assumes that we'll never
> ever add more scripting languages to the bindings. I'd say it would
> be safer to explicitly check for python before changing the extension
> to ".pyd". We don't want the same problem to bite us again some time
> later. We should rather make sure that this problem will never hit
> us again. So I'd suggest doing something like this instead:
>
> if self.lang == "ruby":
> lib_filename = module_name + lib_extension
> elif self.lang == "perl":
> lib_filename = '_' + module_name.capitalize() + lib_extension
> elif self.lang == "python":
> lib_extension = self.gen_obj._extension_map['pyd', 'target']
> else:
> # throw an error, unknown language binding, please add proper
> # library filename extension here
>
> Stefan
>
Hi,
Its not that I think this is a bad idea, but I'm trying to follow the patch
submission guidelines: "A patch submission should contain one logical change;
please don't mix N unrelated changes in one submission — send N separate emails
instead." I am only trying to restore my ability to build the Ruby bindings
without local modifications. Since I haven't been able to build the Perl
bindings for several years, I was actually quite hesitant to include
that in the
patch. The last time I suggested patching something to do with the Perl I was
told I shouldn't since I wasn't a Perl expert.
With all that in mind, I was trying to make a very small change that would
accomplish my goal. I would prefer that my patch or similar be approved and
then you make whatever changes you think best to the formatting and logic.
--
Joe
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2359276
Re: [Patch] Make Ruby and Perl shared libraries .dll again
Posted by Stefan Sperling <st...@elego.de>.
On Thu, May 28, 2009 at 10:23:54PM -0700, Joe Swatosh wrote:
> This is not part of the Ruby bindings so it is not for me to commit
> without approval by a full committer. Is the patch acceptable with the
> log message amended below?
>
> --
> Joe
>
> [[[
>
> Partially revert r37331 "On Windows, shared libs for Python bindings
> should be *.pyd not *.dll." In addition to changing the shared libs for
> the Python bindings, r37331 inadvertently changed the extensions of the
> shared libraries produced for the Perl and Ruby bindings from .dll to
> .pyd. Change the extensions back to .dll for the Ruby and Perl
> bindings.
>
> * build/generator/gen_base.py (TargetSWIG.add_dependencies): Mark Python
> wrapper shared libs as type "pyd" not "lib".
>
> ]]]
>
> ===================================================================
> --- build/generator/gen_base.py (revision 37787)
> +++ build/generator/gen_base.py (working copy)
> @@ -550,12 +550,13 @@
> # Extract SWIG module name from .i file name
> module_name = iname[:4] != 'svn_' and iname[:-2] or iname[4:-2]
>
> - lib_extension = self.gen_obj._extension_map['pyd', 'target']
> + lib_extension = self.gen_obj._extension_map['lib', 'target']
> if self.lang == "ruby":
> lib_filename = module_name + lib_extension
> elif self.lang == "perl":
> lib_filename = '_' + module_name.capitalize() + lib_extension
> else:
> + lib_extension = self.gen_obj._extension_map['pyd', 'target']
Why put this in an else clause? This code assumes that we'll never
ever add more scripting languages to the bindings. I'd say it would
be safer to explicitly check for python before changing the extension
to ".pyd". We don't want the same problem to bite us again some time
later. We should rather make sure that this problem will never hit
us again. So I'd suggest doing something like this instead:
if self.lang == "ruby":
lib_filename = module_name + lib_extension
elif self.lang == "perl":
lib_filename = '_' + module_name.capitalize() + lib_extension
elif self.lang == "python":
lib_extension = self.gen_obj._extension_map['pyd', 'target']
else:
# throw an error, unknown language binding, please add proper
# library filename extension here
Stefan
Re: [Patch] Make Ruby and Perl shared libraries .dll again
Posted by Joe Swatosh <jo...@gmail.com>.
On Wed, Oct 21, 2009 at 8:02 AM, David James <ja...@gmail.com> wrote:
> On Thu, May 28, 2009 at 10:23 PM, Joe Swatosh <jo...@gmail.com> wrote:
>> This is not part of the Ruby bindings so it is not for me to commit
>> without approval by a full committer. Is the patch acceptable with the
>> log message amended below?
>
> Hi Joe,
>
> Your patch looks good. I haven't tested it, but it sounds like you
> have, so +1 to commit.
>
Thanks David. r40151.
--
Joe
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2409829
Re: [Patch] Make Ruby and Perl shared libraries .dll again
Posted by David James <ja...@gmail.com>.
On Thu, May 28, 2009 at 10:23 PM, Joe Swatosh <jo...@gmail.com> wrote:
> This is not part of the Ruby bindings so it is not for me to commit
> without approval by a full committer. Is the patch acceptable with the
> log message amended below?
Hi Joe,
Your patch looks good. I haven't tested it, but it sounds like you
have, so +1 to commit.
Cheers,
David
>
> --
> Joe
>
> [[[
>
> Partially revert r37331 "On Windows, shared libs for Python bindings
> should be *.pyd not *.dll." In addition to changing the shared libs for
> the Python bindings, r37331 inadvertently changed the extensions of the
> shared libraries produced for the Perl and Ruby bindings from .dll to
> .pyd. Change the extensions back to .dll for the Ruby and Perl
> bindings.
>
> * build/generator/gen_base.py (TargetSWIG.add_dependencies): Mark Python
> wrapper shared libs as type "pyd" not "lib".
>
> ]]]
>
> ===================================================================
> --- build/generator/gen_base.py (revision 37787)
> +++ build/generator/gen_base.py (working copy)
> @@ -550,12 +550,13 @@
> # Extract SWIG module name from .i file name
> module_name = iname[:4] != 'svn_' and iname[:-2] or iname[4:-2]
>
> - lib_extension = self.gen_obj._extension_map['pyd', 'target']
> + lib_extension = self.gen_obj._extension_map['lib', 'target']
> if self.lang == "ruby":
> lib_filename = module_name + lib_extension
> elif self.lang == "perl":
> lib_filename = '_' + module_name.capitalize() + lib_extension
> else:
> + lib_extension = self.gen_obj._extension_map['pyd', 'target']
> lib_filename = '_' + module_name + lib_extension
>
> self.name = self.lang + '_' + module_name
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2356664
>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2409816
Re: [Patch] Make Ruby and Perl shared libraries .dll again
Posted by Joe Swatosh <jo...@gmail.com>.
On Fri, May 22, 2009 at 11:07 AM, Branko Čibej <br...@xbc.nu> wrote:
> Joe Swatosh wrote:
>> Hi Mark, Stefan, Brane
>>
>> On Fri, May 22, 2009 at 6:55 AM, Mark Phippard <ma...@gmail.com> wrote:
>>
>>> On Fri, May 22, 2009 at 9:52 AM, Stefan Sperling <st...@elego.de> wrote:
>>>
>>>> On Thu, May 21, 2009 at 11:26:50PM -0700, Joe Swatosh wrote:
>>>>
>>>>> Make the Ruby and Perl shared libraries .dll instead of .pyd. There is
>>>>> probably a smarter way to do this with the ._extension_map, but I'm no
>>>>> kind of Python expert.
>>>>>
>>>> Your log message and explanation of your patch do not contain any
>>>> reasoning about why r37331 needs to be reverted. Can you explain?
>>>> You seem to assume that everyone already knows what's wrong with
>>>> .pyd. But, for example, I don't even know what a .pyd is! :)
>>>>
>>>> Please try to explain the reasoning behind a change in the log message.
>>>>
>>> I had the same reaction. I suspect that it is tied to the comment
>>> about Ruby. Clearly, the Ruby bindings DLL should not be named .pyd
>>> as that is a Python-specific extension. So perhaps the problem is
>>> that the way this change was implemented it impacts the Ruby bindings?
>>>
>>>
>>
>> Hm. I guess I was having a hard time yesterday saying what I mean. Maybe
>> it was just too late. That is it exactly. r37331 changed the extensions of the
>> shared libraries for all the SWIG based bindings to .pyd. Truthfully, I don't
>> know what a .pyd is either, but it prevented the Ruby (and I am assuming)
>> the Perl bindings from working.
>>
>
> That'd be my bad ... I didn't check a patch carefully enough. BTW, a
> .pyd is a .dll containing a Python extension, on Windows.
>
This is not part of the Ruby bindings so it is not for me to commit
without approval by a full committer. Is the patch acceptable with the
log message amended below?
--
Joe
[[[
Partially revert r37331 "On Windows, shared libs for Python bindings
should be *.pyd not *.dll." In addition to changing the shared libs for
the Python bindings, r37331 inadvertently changed the extensions of the
shared libraries produced for the Perl and Ruby bindings from .dll to
.pyd. Change the extensions back to .dll for the Ruby and Perl
bindings.
* build/generator/gen_base.py (TargetSWIG.add_dependencies): Mark Python
wrapper shared libs as type "pyd" not "lib".
]]]
===================================================================
--- build/generator/gen_base.py (revision 37787)
+++ build/generator/gen_base.py (working copy)
@@ -550,12 +550,13 @@
# Extract SWIG module name from .i file name
module_name = iname[:4] != 'svn_' and iname[:-2] or iname[4:-2]
- lib_extension = self.gen_obj._extension_map['pyd', 'target']
+ lib_extension = self.gen_obj._extension_map['lib', 'target']
if self.lang == "ruby":
lib_filename = module_name + lib_extension
elif self.lang == "perl":
lib_filename = '_' + module_name.capitalize() + lib_extension
else:
+ lib_extension = self.gen_obj._extension_map['pyd', 'target']
lib_filename = '_' + module_name + lib_extension
self.name = self.lang + '_' + module_name
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2356664
Re: [Patch] Make Ruby and Perl shared libraries .dll again
Posted by Branko Cibej <br...@xbc.nu>.
Joe Swatosh wrote:
> Hi Mark, Stefan, Brane
>
> On Fri, May 22, 2009 at 6:55 AM, Mark Phippard <ma...@gmail.com> wrote:
>
>> On Fri, May 22, 2009 at 9:52 AM, Stefan Sperling <st...@elego.de> wrote:
>>
>>> On Thu, May 21, 2009 at 11:26:50PM -0700, Joe Swatosh wrote:
>>>
>>>> Make the Ruby and Perl shared libraries .dll instead of .pyd. There is
>>>> probably a smarter way to do this with the ._extension_map, but I'm no
>>>> kind of Python expert.
>>>>
>>> Your log message and explanation of your patch do not contain any
>>> reasoning about why r37331 needs to be reverted. Can you explain?
>>> You seem to assume that everyone already knows what's wrong with
>>> .pyd. But, for example, I don't even know what a .pyd is! :)
>>>
>>> Please try to explain the reasoning behind a change in the log message.
>>>
>> I had the same reaction. I suspect that it is tied to the comment
>> about Ruby. Clearly, the Ruby bindings DLL should not be named .pyd
>> as that is a Python-specific extension. So perhaps the problem is
>> that the way this change was implemented it impacts the Ruby bindings?
>>
>>
>
> Hm. I guess I was having a hard time yesterday saying what I mean. Maybe
> it was just too late. That is it exactly. r37331 changed the extensions of the
> shared libraries for all the SWIG based bindings to .pyd. Truthfully, I don't
> know what a .pyd is either, but it prevented the Ruby (and I am assuming)
> the Perl bindings from working.
>
That'd be my bad ... I didn't check a patch carefully enough. BTW, a
.pyd is a .dll containing a Python extension, on Windows.
-- Brane
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2352959
Re: [Patch] Make Ruby and Perl shared libraries .dll again
Posted by Joe Swatosh <jo...@gmail.com>.
Hi Mark, Stefan, Brane
On Fri, May 22, 2009 at 6:55 AM, Mark Phippard <ma...@gmail.com> wrote:
> On Fri, May 22, 2009 at 9:52 AM, Stefan Sperling <st...@elego.de> wrote:
>> On Thu, May 21, 2009 at 11:26:50PM -0700, Joe Swatosh wrote:
>>> Make the Ruby and Perl shared libraries .dll instead of .pyd. There is
>>> probably a smarter way to do this with the ._extension_map, but I'm no
>>> kind of Python expert.
>>
>> Your log message and explanation of your patch do not contain any
>> reasoning about why r37331 needs to be reverted. Can you explain?
>> You seem to assume that everyone already knows what's wrong with
>> .pyd. But, for example, I don't even know what a .pyd is! :)
>>
>> Please try to explain the reasoning behind a change in the log message.
>
> I had the same reaction. I suspect that it is tied to the comment
> about Ruby. Clearly, the Ruby bindings DLL should not be named .pyd
> as that is a Python-specific extension. So perhaps the problem is
> that the way this change was implemented it impacts the Ruby bindings?
>
Hm. I guess I was having a hard time yesterday saying what I mean. Maybe
it was just too late. That is it exactly. r37331 changed the extensions of the
shared libraries for all the SWIG based bindings to .pyd. Truthfully, I don't
know what a .pyd is either, but it prevented the Ruby (and I am assuming)
the Perl bindings from working.
[[[
Partially revert r37331 "On Windows, shared libs for Python bindings
should be *.pyd not *.dll."
It appears renaming the shared libraries for Perl and Ruby was
inadvertent, so change the extension back to .dll for Ruby and Perl.
* build/generator/gen_base.py (TargetSWIG.add_dependencies): Mark Python
wrapper shared libs as type "pyd" not "lib".
]]]
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2352916
Re: [Patch] Make Ruby and Perl shared libraries .dll again
Posted by Mark Phippard <ma...@gmail.com>.
On Fri, May 22, 2009 at 9:52 AM, Stefan Sperling <st...@elego.de> wrote:
> On Thu, May 21, 2009 at 11:26:50PM -0700, Joe Swatosh wrote:
>> Make the Ruby and Perl shared libraries .dll instead of .pyd. There is
>> probably a smarter way to do this with the ._extension_map, but I'm no
>> kind of Python expert.
>
> Your log message and explanation of your patch do not contain any
> reasoning about why r37331 needs to be reverted. Can you explain?
> You seem to assume that everyone already knows what's wrong with
> .pyd. But, for example, I don't even know what a .pyd is! :)
>
> Please try to explain the reasoning behind a change in the log message.
I had the same reaction. I suspect that it is tied to the comment
about Ruby. Clearly, the Ruby bindings DLL should not be named .pyd
as that is a Python-specific extension. So perhaps the problem is
that the way this change was implemented it impacts the Ruby bindings?
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2352903
Re: [Patch] Make Ruby and Perl shared libraries .dll again
Posted by Stefan Sperling <st...@elego.de>.
On Thu, May 21, 2009 at 11:26:50PM -0700, Joe Swatosh wrote:
> Make the Ruby and Perl shared libraries .dll instead of .pyd. There is
> probably a smarter way to do this with the ._extension_map, but I'm no
> kind of Python expert.
Your log message and explanation of your patch do not contain any
reasoning about why r37331 needs to be reverted. Can you explain?
You seem to assume that everyone already knows what's wrong with
.pyd. But, for example, I don't even know what a .pyd is! :)
Please try to explain the reasoning behind a change in the log message.
Thanks,
Stefan
>
> [[[
>
> Partially revert r37331 "On Windows, shared libs for Python bindings
> should be *.pyd not *.dll." Change back to .dll for Ruby and Perl.
>
> * build/generator/gen_base.py (TargetSWIG.add_dependencies): Mark Python
> wrapper shared libs as type "pyd" not "lib".
>
> ]]]
>
> ===================================================================
> --- build/generator/gen_base.py (revision 37787)
> +++ build/generator/gen_base.py (working copy)
> @@ -550,12 +550,13 @@
> # Extract SWIG module name from .i file name
> module_name = iname[:4] != 'svn_' and iname[:-2] or iname[4:-2]
>
> - lib_extension = self.gen_obj._extension_map['pyd', 'target']
> + lib_extension = self.gen_obj._extension_map['lib', 'target']
> if self.lang == "ruby":
> lib_filename = module_name + lib_extension
> elif self.lang == "perl":
> lib_filename = '_' + module_name.capitalize() + lib_extension
> else:
> + lib_extension = self.gen_obj._extension_map['pyd', 'target']
> lib_filename = '_' + module_name + lib_extension
>
> self.name = self.lang + '_' + module_name
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2352771