Skip to content
This repository has been archived by the owner on Mar 7, 2019. It is now read-only.

add bin (if it exists) directory to PATH on win32 #32

Closed
wants to merge 2 commits into from
Closed

add bin (if it exists) directory to PATH on win32 #32

wants to merge 2 commits into from

Conversation

plicease
Copy link
Contributor

in order to use .dlls on MSWin32 and cygwin, they must be in the PATH. This patch adds the bin directory, where the dlls are normally placed, if it exists.

The only downside to this I can see is that there may be some unwanted .exe files in the bin directory as well, at the cost of some complexity, it may pay do one of:

  • make this optional (though windows dll won't work without it)
  • remove .exe files from bin
  • copy/move .dll files to an alternate directory and put THAT directory in the PATH

in order to use the .dll on MSWin32 and cygwin, it must be in the PATH
@jberger
Copy link
Member

jberger commented Dec 15, 2013

I don't believe this is necessary as I am directly loading the files using DynaLoader. My test cases (see Acme::Alien::DontPanic and Acme::FordPrefect) pass on windows.

@plicease
Copy link
Contributor Author

I believe I needed this for Alien::Libarchive, I will take a look and see if it is specific to that case.

@plicease
Copy link
Contributor Author

I wasn't able to get Acme::Alien::DontPanic to install on either Strawberry Perl (MSWin32) or Cygwin, but it did build on cygwin. I didn't see any .dll files, just this static lib:

fs01% pwd
/cygdrive/d/dev/Acme-Alien-DontPanic-0.005/_alien/dontpanic/src/.libs
fs01% ls
libdontpanic.a  libdontpanic.la@  libdontpanic.lai  libdontpanic_la-libdontpanic.o
fs01% 

I wouldn't expect that to exercise the behavior that I am seeing. Or maybe that is a dll export library and the .dll was missing? I am not sure. This is the error in Cygwin that I am seeing:

t/use.t .................................. 1/1 
#   Failed test 'use Archive::Libarchive::XS;'
#   at t/use.t line 5.
#     Tried to use 'Archive::Libarchive::XS'.
#     Error:  Can't load '/cygdrive/d/dev/Archive-Libarchive-XS/Archive-Libarchive-XS-0.01/blib/arch/auto/Archive/Libarchive/XS/XS.dll' for module Archive::Libarchive::XS: No such file or directory at /usr/lib/perl5/5.14/i686-cygwin-threads-64int/DynaLoader.pm line 190.
#  at /cygdrive/d/dev/Archive-Libarchive-XS/Archive-Libarchive-XS-0.01/blib/lib/Archive/Libarchive/XS.pm line 20.
# BEGIN failed--compilation aborted at /cygdrive/d/dev/Archive-Libarchive-XS/Archive-Libarchive-XS-0.01/blib/lib/Archive/Libarchive/XS.pm line 20.
# Compilation failed in require at (eval 4) line 2.
# BEGIN failed--compilation aborted at (eval 4) line 2.
# Looks like you failed 1 test of 1.

cygcheck reveals that it can't find cygarchive-13.dll

fs01% cygcheck /cygdrive/d/dev/Archive-Libarchive-XS/Archive-Libarchive-XS-0.01/blib/arch/auto/Archive/Libarchive/XS/XS.dll 
d:\dev\Archive-Libarchive-XS\Archive-Libarchive-XS-0.01\blib\arch\auto\Archive\Libarchive\XS\XS.dll
  D:\cygwin\bin\cygwin1.dll
    C:\Windows\system32\KERNEL32.dll
      C:\Windows\system32\API-MS-Win-Core-RtlSupport-L1-1-0.dll
      C:\Windows\system32\ntdll.dll
      C:\Windows\system32\KERNELBASE.dll
      C:\Windows\system32\API-MS-Win-Core-ProcessThreads-L1-1-0.dll
      C:\Windows\system32\API-MS-Win-Core-Heap-L1-1-0.dll
      C:\Windows\system32\API-MS-Win-Core-Memory-L1-1-0.dll
      C:\Windows\system32\API-MS-Win-Core-Handle-L1-1-0.dll
      C:\Windows\system32\API-MS-Win-Core-Synch-L1-1-0.dll
      C:\Windows\system32\API-MS-Win-Core-File-L1-1-0.dll
      C:\Windows\system32\API-MS-Win-Core-IO-L1-1-0.dll
      C:\Windows\system32\API-MS-Win-Core-ThreadPool-L1-1-0.dll
      C:\Windows\system32\API-MS-Win-Core-LibraryLoader-L1-1-0.dll
      C:\Windows\system32\API-MS-Win-Core-NamedPipe-L1-1-0.dll
      C:\Windows\system32\API-MS-Win-Core-Misc-L1-1-0.dll
      C:\Windows\system32\API-MS-Win-Core-SysInfo-L1-1-0.dll
      C:\Windows\system32\API-MS-Win-Core-Localization-L1-1-0.dll
      C:\Windows\system32\API-MS-Win-Core-ProcessEnvironment-L1-1-0.dll
      C:\Windows\system32\API-MS-Win-Core-String-L1-1-0.dll
      C:\Windows\system32\API-MS-Win-Core-Debug-L1-1-0.dll
      C:\Windows\system32\API-MS-Win-Core-ErrorHandling-L1-1-0.dll
      C:\Windows\system32\API-MS-Win-Core-Fibers-L1-1-0.dll
      C:\Windows\system32\API-MS-Win-Core-Util-L1-1-0.dll
      C:\Windows\system32\API-MS-Win-Core-Profile-L1-1-0.dll
      C:\Windows\system32\API-MS-Win-Security-Base-L1-1-0.dll
  D:\cygwin\bin\cygperl5_14.dll
    D:\cygwin\bin\cygcrypt-0.dll
    D:\cygwin\bin\cyggcc_s-1.dll
    D:\cygwin\bin\cygssp-0.dll
cygcheck: track_down: could not find cygarchive-13.dll

on MSWin32 (Strawberry) I get a GUI error "The program can't start because libarchive.dll is missing from your computer. Try reinstalling the program to fix this problem". I am doing some funny contortions on MSWin32 because it uses CBuilder instead of autoconf, but the cygwin version is mostly vanilla.

btw- this is what I get trying to install Acme::Alien::DontPanic:

d:\dev\Acme-Alien-DontPanic-0.005>Build
Building Acme-Alien-DontPanic
Downloading File: dontpanic-1.0.tar.gz ... Done
Extracting Archive ... Done
Building library ... 'configure' is not recognized as an internal or external command,
operable program or batch file.
External command (configure --prefix=D:\perl5\native\lib\perl5\auto\share\dist\Acme-Alien-DontPanic) failed! Error: 256
 at Build line 59.
Failed
Build not completed at Build line 59.

How are you building on Windows? I wouldn't expect that to work without something like MSYS, and even then, I would expect "configure" to be written "sh configure" there. If this is supposed to work some documentation on how to get it to work on windows would be helpful.

This is the test failure on cygwin:

fs01% ./Build test
t/00-use.t .. 1/1 
#   Failed test 'idenitified needed library'
#   at t/00-use.t line 12.
#          got: undef
#     expected: 'dontpanic'
# Looks like you failed 1 test of 1.
t/00-use.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/1 subtests 

Test Summary Report
-------------------
t/00-use.t (Wstat: 256 Tests: 1 Failed: 1)
  Failed test:  1
  Non-zero exit status: 1
Files=1, Tests=1,  1 wallclock secs ( 0.03 usr  0.00 sys +  0.11 cusr  0.09 csys =  0.23 CPU)
Result: FAIL
Failed 1/1 test programs. 1/1 subtests failed.

@mohawk2
Copy link
Contributor

mohawk2 commented Aug 30, 2014

@plicease, with the benefit of a little separation in time, may I ask you to review your own code, and then merge if you feel it's ready?

@plicease
Copy link
Contributor Author

I am not 100% on board with this solution at the moment. It's not a terrible solution, but if there are binaries installed in that bin directory in addition to the .dll files then it changes the behavior of the environment in subtle ways. The way I ended up solving this in Alien::Libarchive was actually to use static libraries (#11). The problem is that at the moment the only solution that I see for #11 requires some intervention on the part of an author using Alien::Base.

@mohawk2
Copy link
Contributor

mohawk2 commented Aug 30, 2014

If it requires a hook from an A::B subclasser, we could add an interface for that? What kind of hooks might be required?

@plicease
Copy link
Contributor Author

I am not sure I follow what you mean.

@mohawk2
Copy link
Contributor

mohawk2 commented Aug 30, 2014

The problem is that at the moment the only solution that I see for #11 requires some intervention on the part of an author using Alien::Base.

The "intervention" I'm assuming could in a perfect world be dealt with by passing a code-ref in some fashion?

@jberger
Copy link
Member

jberger commented Aug 30, 2014

sadly, no code-refs are allowed. This is a limitation on the way all Perl build systems serialize their state to support the multiphase build process (configure, build, test, install, etc). I wish it weren't so, but there it is.

@mohawk2
Copy link
Contributor

mohawk2 commented Aug 30, 2014

So one could pass text that could be evaluated to be a code-ref? I don't know MB at all well, but I think I saw that in a Build.PL today.

@plicease
Copy link
Contributor Author

Subclassing may also be an option.

@jberger
Copy link
Member

jberger commented Aug 30, 2014

The lack of state inherent in this build style of Perl modules was actually motivating me to make my own attempt at a Module::Build reimplementation. It had a central module that would get loaded each time (which could register callbacks/hooks by name), state could still be serialized, but it wouldn't be the only way that the system operates in later phases. In this way, when an event was emitted, it could call user-specified callbacks. Short of this, I don't think it can be done.

@plicease
Copy link
Contributor Author

plicease commented Sep 4, 2014

I think that a better approach is #51 and #47 to use static libs for XS, so I am closing this PR.

@plicease plicease closed this Sep 4, 2014
@devel-chm
Copy link

I don't see how forcing static libraries could possibly work on
windows since many of the existing libraries are released as
dlls and not as static libraries.

If upgraded libraries are a problem then the Alien::XXX implementor
should definitely be able to use and be encouraged to use
static libraries.

However, if the libraries are only distributed as DLLs, then the
Alien::XXX needs the dynamic libraries to work. There are a lot
of other platforms that seem to make dynamic libraries work so
forcing static seems to be a false simplification to me.

This seems a reasonable approach only for platforms where
you have access to build libraries and utilities from scratch.
This definitely hurts windows and I can't imagine that really
makes things wonderful every other platform. Even cygwin
uses DLLs.

Recommend the ticket is still relevant.

On Thu, Sep 4, 2014 at 8:10 AM, Graham Ollis notifications@github.com
wrote:

I think that a better approach is #51
#51 and #47
#47 to use static libs
for XS, so I am closing this PR.

@plicease
Copy link
Contributor Author

plicease commented Sep 4, 2014

@devel-chm

DLLs in the share directory should be discouraged since an upgrade to Alien::Foo can break any Foo::XS that depends on it. This PR only addresses that situation.

@plicease
Copy link
Contributor Author

plicease commented Sep 4, 2014

@devel-chm

I think part of the disconnect here is that most of the libraries that I care about can be built pretty easily from source, even on windows with the new Alien::MSYS support that we have added. Maybe if you gave me an example lib or two where you think this would be an issue I can come up with some better solutions than what is provided by this PR.

@devel-chm
Copy link

Pretty much any standard MS library, other vendors libraries,
interface libraries for hardware devices for example, any library
that isn't open source---some are freely available but not
source available. In other cases it may be possible to build
the win32 library but not with opensource toolchain....

On Thu, Sep 4, 2014 at 6:38 PM, Graham Ollis notifications@github.com
wrote:

@devel-chm https://github.com/devel-chm

I think part of the disconnect here is that most of the libraries that I
care about can be built pretty easily from source, even on windows with the
new Alien::MSYS support that we have added. Maybe if you gave me an
example lib or two where you think this would be an issue I can come up
with some better solutions than what is provided by this PR.


Reply to this email directly or view it on GitHub
#32 (comment).

@plicease
Copy link
Contributor Author

plicease commented Sep 4, 2014

@devel-chm

Okay, but some specific examples would help so that we can start addressing them. Again, 99% of the stuff I care about is handled by building from source.

@plicease
Copy link
Contributor Author

plicease commented Sep 5, 2014

@devel-chm

btw- I am not trying to be flippant or rude or anything, I am just saying that we work in different worlds. Linux and open source is where I do most of my coding and Windows is a sort of interesting sideline. If you gave me one or two specific libraries, preferably something that would be useful for PDL or something that would have an actual customer, then we could start working on specific issues and work toward some general solutions.

@jberger
Copy link
Member

jberger commented Sep 5, 2014

I expect that darn near everything on this page is distributed as compiled libraries (of wildly varying quality I might add).

@plicease
Copy link
Contributor Author

plicease commented Sep 5, 2014

That page requires login to download the few things that I tried. That isn't going to easily work with install_type=share anyway. I imagine that you could get it to work with install_type=system depending on how you do the detection logic.

@zmughal
Copy link
Member

zmughal commented Sep 5, 2014

It seems to me that we should treat dynamic-only libraries on Windows as a special case. (It seems that's what many build systems are—lots and lots of special cases.)

I think that @plicease's original suggestion can still be used:

  • copy/move .dll files to an alternate directory and put THAT directory in the PATH
    would be best.

The first part is already done with alien_isolate_dynamic. If we made a way to choose whether or not it should be added to the path, that would make for a flexible solution. Something like:

For users of Alien::Foo:

use Alien::Foo dll_path => 0;

and in Alien::Base::import:

    $_{dll_path} ||= 1;
    if( $win32 && $dll_only && -d "$dist_dir/dynamic" && $_{dll_path} ) {
        add_to_path();
    }

@plicease
Copy link
Contributor Author

plicease commented Sep 5, 2014

@zmughal

That addresses the problem where extraneous .exe files are in the bin directory, but not the problem that upgrades to Alien::Foo can pretty easily break already installed Foo::XS installs. It is probably a reasonable as an optional features with documented caveats.

One solution would be to install to share/timestamp/{bin,lib,include} where timestamp is unique for each install of Alien::Foo. XS::Foo would have to note the location of the DLLs at build time and use that at runtime regardless of Alien::Foo upgrades. Downsides include installing a whole extra version every time you upgrade/reinstall Alien::Foo and that Foo1::XS and Foo2::XS that were built against different versions of Alien::Foo will probably not work together. With documented caveats it may be a reasonable optional feature useful for some packages.

However, I earnestly believe that we need a concrete and specific example of a library that only comes with DLLs and no source that could be downloaded automatically and installed in an install_type=share mode to exercise development of any such features.

@mohawk2
Copy link
Contributor

mohawk2 commented Sep 5, 2014

Sounds like we should make a new Acme module?

@zmughal
Copy link
Member

zmughal commented May 23, 2015

I found one example that comes with source, but does not build static libraries. I just got Alien::ZMQ to compile under Windows (see chazmcgarvey/p5-Alien-ZMQ#5), but the build throws back

configure: error: Building static libraries is not supported under MinGW32

when I run ./configure --enable-static.

P.S. Alien::ZMQ does not use Alien::Base.

@plicease
Copy link
Contributor Author

@zmughal, I intended to respond a while ago. Here is the commit where they disabled static builds:

zeromq/libzmq@9255952

It isn't critical right this moment, since as you say Alien::ZMQ does not use AB. I would support an OPTION to add the .dlls to the PATH on windows. It should not be on by default, and the documentation for the option should clearly explain the pitfalls. I would prefer not to add any .exes unless they were explicitly asked for (there is a separate method for that now). If we did have an AB based Alien that was trying to support a libfoo that lacked support for static libs on MSWin32, I would like to see at least some push back on the libfoo developer to see if they can support static builds.

@zmughal
Copy link
Member

zmughal commented Jun 19, 2015

I do need it for Alien::ZMQ4 (which I made because Alien::ZMQ just supports libzmq3 and not libzmq4). I want to get it working on Windows before I make a CPAN release.

Right, I agree on having push back on the developers and I did see the following on the ZeroMQ MSWin32 installer page:

static libraries are set to appear in the next stable revision

so this may be resolved soon.

Also, for the cases where one does try to add the DLL to the path, it might be a good idea to have a warning that suggests the Alien package author use the alien_isolate_dynamic option.

@jberger
Copy link
Member

jberger commented Jun 19, 2015

I don't mind the idea of modifying the $PATH, in fact this is how Alien::Base::import used to work. That said, I agree with @plicease that unless this situation can be detected and limited to only the needed case, I would want there to be an option. Modifying the $PATH on all windows systems for all libraries is too much.

Also, sidenote that I would like to see more generic usage of platform conforming code rather than the hardcoded per-platform path and PATH separators. See $Config{path_sep} or Env module for PATH separators and of course File::Spec for path joining.

@plicease
Copy link
Contributor Author

@jberger agree with everything you said here. Just to clarify I was suggesting only modifying the PATH for the Perl process, not system wide.

@plicease plicease deleted the cygwin_and_win32 branch July 13, 2015 13:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants