[PATCH] Issue 1627: Create subversion DLLs on Windows

[Vlad Georgescu <vg...@gmail.com>]

This patch changes the build system on Windows as follows:

* For each of our main libraries (except libsvn_ra_foo and
libsvn_fs_bar) we build a static library named svn_foo-1.lib and a dll
called libsvn_foo-1.dll (with an associated libsvn_foo-1.lib import
library). The dlls are built by simply relinking the static libraries,
not by recompiling. The naming is consistent with how APR names its
libraries on Windows.

* The list of exported symbols is obtained by parsing the header files
with the extractor.py script, to which I added a few special cases
(see below).

* In Visual Studio, the static projects are still named libsvn_foo
while the dll projects are named libsvn_foo_dll.

* The programs (everything except tests) are linked against the dlls.
Tests are still linked statically because some of them access internal
library functions.

* gen_make.py supports a --disable-shared option for backward compatibility.

To get this to work correctly, and with 100% backward compatibility, I
had to jump through quite a few hoops:

* Exporting variables. Currently there is only one variable in our
public header files: svn_ctype_table. We can't export this variable
directly, because the VS linker adds a pointer indirection to each
exported variable[1]. However, it turns out that if we export the
actual global array defined in ctype.c (i.e. the variable containing
the array, not a pointer to it), the extra pointer indirection gives
us exactly what we want. It works like this:

In ctype.c:
const apr_uint32_t svn_ctype_table_internal[256] = { // table data };

In the .def file:
svn_ctype_table = svn_ctype_table_internal CONSTANT

* svn_ra_dav_init and svn_ra_serf_init. These (deprecated) functions
are declared in svn_ra.h. Depending on how you build Subversion, only
one of these will be defined, so we can't export both. To handle this,
I added some extra logic to extractor.py to determine which of neon
and serf is linked in and, based on that decision, only export the
appropriate function.

* Visual Studio refuses to build a project's outputs if a project has
no source files, as is the case with our dll projects. As a
workaround, I added an empty.c file to each such project.

Note: I only tested this patch with Visual Studio 2005. It would
probably be trivial to make it work with VC++ 6.0.

The changes to build.conf and win-tests.py are based on Russell
Yanofsky's patches attached to issue 1627.

[[[
Support building DLLs on Windows. This closes #1627.

Patch by: Vlad Georgescu <vg...@gmail.com>
          rey4

* build.conf:
  (libsvn_client, libsvn_delta, libsvn_diff, libsvn_fs,
   libsvn_ra, libsvn_repos, libsvn_subr, libsvn_wc):
  Replace the msvc_static option with a msvc_export option containing the
  list of public header files for that library.

* build/generator/extractor.py:
  Handle svn_ctype_table and svn_ra_{serf,dav}_init.
  (_filter_names): Add svn_auth_get_keychain_simple_provider,
   svn_ra_serf_init and svn_ra_dav_init.

* build/generator/gen_win.py
  (create_dll_target): New. Given a static library, it creates a new
   dynamic library target that depends on that library.
  (get_install_targets): Call create_dll_target for each target that has
   msvc_export set.
  (get_linked_win_depends): Scan the dependency tree in breadth-first
   order, allowing us to prevent cases when both a dll and its
   corresponding static library are linked into o project.

* build/generator/vcnet_vcproj.ezt:
  Add empty.c as a source file for dll projects, because Visual Studio
  refuses to build a project's outputs if it has no source files.

* build/win32/empty.c: New.

* gen-make.py:
  Handle the new --disable-shared option.

* subversion/libsvn_subr/ctype.c
  (svn_ctype_table_internal): Rename from ctype_table. Remove 'static'
   qualifier.

* win-tests.py:
  Append the paths to our libraries' dlls to the PATH environment variable.
]]]

[1] http://svn.haxx.se/dev/archive-2005-12/0284.shtml

-- 
Vlad

Re: [PATCH] Issue 1627: Create subversion DLLs on Windows

[Branko Čibej <br...@xbc.nu>]

Committed the latest version of this patch, with necessary tweaks to
make it work with MSVC 6, in r22841. Thanks, thanks, thanks! This looks
most marvelous.

The thing to do before branching 1.5 is to update
build/win32/gen_dist.py to take the new DLLs into account, and of course
to update the Windows installer.

-- Brane

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Issue 1627: Create subversion DLLs on Windows

[Malcolm Rowe <ma...@farside.org.uk>]

On Mon, Dec 18, 2006 at 06:02:18PM +0200, Vlad Georgescu wrote:
> Good idea. I've added stubs like this to libsvn_ra/ra_loader.c, one
> for each RA library:
> 
> #ifndef SVN_LIBSVN_CLIENT_LINKS_RA_FOO
> svn_error_t *
> svn_ra_foo_init(int abi_version,
>                apr_pool_t *pool,
>                apr_hash_t *hash)
> {
>  return svn_error_create(SVN_ERR_RA_NOT_IMPLEMENTED, NULL, NULL);
> }
> #endif
> 
> I attached the updated log message and patch.
> 

Unless anyone objects to that change, I think that's something that can
go in separately, as it's really orthogonal to the DLL patch.  Max and I
discussed adding stubs in the past, but neither of us got around to it.

Regards,
Malcolm

Re: [PATCH] Issue 1627: Create subversion DLLs on Windows

[Vlad Georgescu <vg...@gmail.com>]

On 12/30/06, Branko Čibej <br...@xbc.nu> wrote:
> I have one question about exporting svn_ctype_table. My linker complains
> that the CONSTANT keyword in the .def file is deprecated, and that DATA
> should be used instead. I don't recall the reason for using CONSTANT
> instead of DATA; I thought that the end result was the same?

No. If you export with DATA you still have to use
__declspec(dllimport) to use the variable (at least that's what
happens with VS2005). CONSTANT allows you to use it directly [1].

CONSTANT is more dangerous because it doesn't do the pointer
indirection for you, but in our case, that's exactly what we want.

[1] http://msdn2.microsoft.com/en-us/library/54xsd65y(VS.71).aspx

-- 
Vlad

Re: [PATCH] Issue 1627: Create subversion DLLs on Windows

[Branko Čibej <br...@xbc.nu>]

Vlad Georgescu wrote:
> On 12/18/06, Vlad Georgescu <vg...@gmail.com> wrote:
>> I attached the updated log message and patch.
>
> Two more problems I discovered:
>
> 1. I forgot to add svn_nls.h to the list of header files for
> libsvn_subr. (I don't build with ENABLE_NLS, so I didn't realize this
> until I tried to build the python bindings, which wrap svn_nls_init).
>
> 2. The swig_{client,delta,fs,ra,repos} targets should explicitly
> depend on APR because they use apr_palloc in some places. This worked
> previously because if a project depends on a static library, the
> library's dependencies are recursively added to the dependent project.
> This doesn't happen with shared libs, causing linker errors.
>
> I updated the log message and patch to address these issues.

I finally got the chance to test this. Had to tweak it a bit so that it
works with MSVC 6, but (local) tests are looking great now.

I have one question about exporting svn_ctype_table. My linker complains
that the CONSTANT keyword in the .def file is deprecated, and that DATA
should be used instead. I don't recall the reason for using CONSTANT
instead of DATA; I thought that the end result was the same?

I suspect I'll commit this as soon as the tests pass.

-- Brane

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Issue 1627: Create subversion DLLs on Windows

[Vlad Georgescu <vg...@gmail.com>]

On 12/18/06, Vlad Georgescu <vg...@gmail.com> wrote:
> I attached the updated log message and patch.

Two more problems I discovered:

1. I forgot to add svn_nls.h to the list of header files for
libsvn_subr. (I don't build with ENABLE_NLS, so I didn't realize this
until I tried to build the python bindings, which wrap svn_nls_init).

2. The swig_{client,delta,fs,ra,repos} targets should explicitly
depend on APR because they use apr_palloc in some places. This worked
previously because if a project depends on a static library, the
library's dependencies are recursively added to the dependent project.
This doesn't happen with shared libs, causing linker errors.

I updated the log message and patch to address these issues.

-- 
Vlad

Re: [PATCH] Issue 1627: Create subversion DLLs on Windows

[Vlad Georgescu <vg...@gmail.com>]

On 12/18/06, Branko Čibej <br...@xbc.nu> wrote:
> Vlad Georgescu wrote:
> [...]
>
> > * svn_ra_dav_init and svn_ra_serf_init. These (deprecated) functions
> > are declared in svn_ra.h. Depending on how you build Subversion, only
> > one of these will be defined, so we can't export both. To handle this,
> > I added some extra logic to extractor.py to determine which of neon
> > and serf is linked in and, based on that decision, only export the
> > appropriate function.
>
> I wonder ... this lets us "randomly" create DLLs that might not be
> binary compatible, from the same source. I suspect the Unix build is
> similarly broken.
>
> In a case like this, I'd prefer to always export both functions, and
> only tweak the source to have one of them return APR_ENOTIMPL (or the
> svn_error_t* equivalent).

Good idea. I've added stubs like this to libsvn_ra/ra_loader.c, one
for each RA library:

#ifndef SVN_LIBSVN_CLIENT_LINKS_RA_FOO
svn_error_t *
svn_ra_foo_init(int abi_version,
                apr_pool_t *pool,
                apr_hash_t *hash)
{
  return svn_error_create(SVN_ERR_RA_NOT_IMPLEMENTED, NULL, NULL);
}
#endif

I attached the updated log message and patch.

-- 
Vlad

Re: [PATCH] Issue 1627: Create subversion DLLs on Windows

[Branko Čibej <br...@xbc.nu>]

Vlad Georgescu wrote:
> This patch changes the build system on Windows as follows:

This is absolutely phenomenal work, Vlad! And exactly what I wanted all
along. Thanks! Bravo!

[...]

> * svn_ra_dav_init and svn_ra_serf_init. These (deprecated) functions
> are declared in svn_ra.h. Depending on how you build Subversion, only
> one of these will be defined, so we can't export both. To handle this,
> I added some extra logic to extractor.py to determine which of neon
> and serf is linked in and, based on that decision, only export the
> appropriate function.

I wonder ... this lets us "randomly" create DLLs that might not be
binary compatible, from the same source. I suspect the Unix build is
similarly broken.

In a case like this, I'd prefer to always export both functions, and
only tweak the source to have one of them return APR_ENOTIMPL (or the
svn_error_t* equivalent).

> * Visual Studio refuses to build a project's outputs if a project has
> no source files, as is the case with our dll projects. As a
> workaround, I added an empty.c file to each such project.

Yup. We could even generate such a file ... but keeping a single copy in
build/win32 is just fine (and probably simpler, too).

> Note: I only tested this patch with Visual Studio 2005. It would
> probably be trivial to make it work with VC++ 6.0.
>
> The changes to build.conf and win-tests.py are based on Russell
> Yanofsky's patches attached to issue 1627.

What can I say ... this looks beautiful. And I promise (once again...)
to really, really find time to test this -- like, this year. :)

-- Brane

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org