Den ons 17 juli 2024 kl 16:31 skrev Timofei Zhakov <t...@chemodax.net>:
> On Tue, Jul 16, 2024 at 5:41 PM Daniel Sahlberg
> <daniel.l.sahlb...@gmail.com> wrote:
> >
> > Hi,
> >
> > Inspired by Timofei's work on the cmake branch, I checked it out and
> tried to build on Linux.
> >
> > It worked, almost.
> >
> > [[[
> > [ 17%] Building C object
> CMakeFiles/libsvn_subr.dir/subversion/libsvn_subr/sysinfo.c.o
> > /home/dsg/svn_cmake/subversion/libsvn_subr/sysinfo.c: In function
> ‘linux_release_name’:
> > /home/dsg/svn_cmake/subversion/libsvn_subr/sysinfo.c:649:31: warning:
> implicit declaration of function ‘release_name_from_uname’
> [-Wimplicit-function-declaration]
> > 649 | const char *uname_release = release_name_from_uname(pool);
> > | ^~~~~~~~~~~~~~~~~~~~~~~
> > /home/dsg/svn_cmake/subversion/libsvn_subr/sysinfo.c:649:31: warning:
> initialization of ‘const char *’ from ‘int’ makes pointer from integer
> without a cast [-Wint-conversion]
> > /home/dsg/svn_cmake/subversion/libsvn_subr/sysinfo.c: In function
> ‘linux_shared_libs’:
> > /home/dsg/svn_cmake/subversion/libsvn_subr/sysinfo.c:750:65: warning:
> implicit declaration of function ‘getpid’ [-Wimplicit-function-declaration]
> > 750 | const char *maps = apr_psprintf(pool, "/proc/%ld/maps",
> (long)getpid());
> > |
> ^~~~~~
> > ]]]
> >
> > The above warnings were caused by missing defines HAVE_SYS_UTSNAME_H,
> HAVE_UNAME and HAVE_UNISTD_H.
> >
> > I also got the following error:
> > [[[
> > [ 64%] Linking C shared library liblibsvn_ra-1.so
> > /usr/bin/ld: liblibsvn_ra_svn.a(marshal.c.o): warning: relocation
> against `svn_ctype_table' in read-only section `.text'
> > /usr/bin/ld: liblibsvn_ra_svn.a(client.c.o): relocation R_X86_64_PC32
> against symbol `svn_ctype_table' can not be used when making a shared
> object; recompile with -fPIC
> > /usr/bin/ld: final link failed: bad value
> > collect2: error: ld returned 1 exit status
> > gmake[2]: *** [CMakeFiles/libsvn_ra.dir/build.make:186:
> liblibsvn_ra-1.so] Error 1
> > gmake[1]: *** [CMakeFiles/Makefile2:330: CMakeFiles/libsvn_ra.dir/all]
> Error 2
> > gmake: *** [Makefile:136: all] Error 2
> > ]]]
> >
> > I made some effort trying to figure out the proper way to fix this and I
> came up with the following changes. After this the build succeeds and I get
> a working svn binary.
> >
> > I'm SURE this doesn't follow CMake best-practice but I'd be happy if it
> could be a starting point in getting CMake to work under Linux. I'd also be
> happy to see how it SHOULD be done :-)
> >
> > To get this to work under Macos, someone should look at the
> SVN_HAVE_MACOS_PLIST define.
> >
> > [[[
> > Index: CMakeLists.txt
> > ===================================================================
> > --- CMakeLists.txt (revision 1919275)
> > +++ CMakeLists.txt (working copy)
> > @@ -125,8 +125,29 @@
> > enable_testing()
> > endif()
> >
> > +include(CheckSymbolExists)
> > +include(CheckIncludeFiles)
> > +check_include_files("sys/utsname.h" HAVE_SYS_UTSNAME_H)
> > +if (HAVE_SYS_UTSNAME_H)
> > + add_compile_definitions(HAVE_SYS_UTSNAME_H)
> > +
> > + check_symbol_exists(uname sys/utsname.h HAVE_UNAME)
> > + if (HAVE_UNAME)
> > + add_compile_definitions(HAVE_UNAME)
> > + endif()
> > +endif()
> > +check_include_files("unistd.h" HAVE_UNISTD_H)
> > +if (HAVE_UNISTD_H)
> > + add_compile_definitions(HAVE_UNISTD_H)
> > +endif()
> > +
> > +if (BUILD_SHARED_LIBS)
> > + set(CMAKE_POSITION_INDEPENDENT_CODE ON)
> > +endif()
> > +
> > if(SVN_BUILD_SHARED_FS)
> > set(SVN_FS_BUILD_TYPE SHARED)
> > + set(CMAKE_POSITION_INDEPENDENT_CODE ON)
> > else()
> > set(SVN_FS_BUILD_TYPE STATIC)
> > endif()
> > @@ -133,6 +154,7 @@
> >
> > if(SVN_BUILD_SHARED_RA)
> > set(SVN_RA_BUILD_TYPE SHARED)
> > + set(CMAKE_POSITION_INDEPENDENT_CODE ON)
> > else()
> > set(SVN_RA_BUILD_TYPE STATIC)
> > endif()
> > ]]]
>
> Hi,
>
> Thanks for your attention to the branch and trying to build with
> CMake. I appreciate it.
>
> I was also trying it on Linux and got the same problems, but didn't
> find a nice resolution.
>
> When I was was doing that, I solved the problem with uname with a
> little patch by adding ifdef over release_name_from_uname() invocation
> in sysinfo.c:
>
> [[[
> Index: subversion/libsvn_subr/sysinfo.c
> ===================================================================
> --- subversion/libsvn_subr/sysinfo.c (revision 1919186)
> +++ subversion/libsvn_subr/sysinfo.c (working copy)
> @@ -646,8 +646,12 @@
> static const char *
> linux_release_name(apr_pool_t *pool)
> {
> - const char *uname_release = release_name_from_uname(pool);
> + const char *uname_release;
>
> +#if HAVE_UNAME
> + uname_release = release_name_from_uname(pool);
> +#endif
> +
> /* Try anything that has /usr/bin/lsb_release.
> Covers, for example, Debian, Ubuntu and SuSE. */
> const char *release_name = lsb_release(pool);
> ]]]
>
> This fixed the problem, but the resolution is not nice, because the
> version in svn --version becomes weird. I think I will look at the
> checks using CheckIncludeFiles that you suggested, but they should be
> added for every HAVE_* or SVN_HAVE_* constants, so...
>
Yup, that would fix the problem but change behaviour between CMake and
autconf builds - I don't think that is acceptable.
As you say, all the HAVE_* and SVN_HAVE_* currently set by ./configure
should also be checked and set by CMake.
>
> The problem with svn_ctype_table in that time I fixed by simply
> disabling ra-svn or disabling shared libraries, also adding -fpic to
> compile options resolved the problem. However, I didn't notice the
> CMAKE_POSITION_INDEPENDENT_CODE option and I think I will utilize it.
>
*thumbs up*
>
> Additionally, there are two other problems that I noticed when trying
> to build on Linux and didn't fix yet:
>
> 1. The libraries compiled to the files with double 'lib' prefix and
> there might be files like liblibsvn_subr.so. I'm going to fix it by
> making some modifications in the CMAKE_SHARED_LIBRARY_PREFIX variable
> and also get libraries without the 'lib' prefix in static
> configuration.
>
Oh, yeah, that should be fixed. set(CMAKE_SHARED_LIBRARY_PREFIX "") seems
to do the trick for me but I don't know if there are any downsides.
For the static libraries: set(CMAKE_STATIC_LIBRARY_PREFIX "")
>
> 2. Path separator (SVN_PATH_LOCAL_SEPARATOR) is not currently
> configured correctly, so there might be some runtime problems when
> converting the path from local style and to local style.
>
I saw that one when running the tests. How about:
[[[
Index: subversion/svn_private_config.hc
===================================================================
--- subversion/svn_private_config.hc (revision 1919275)
+++ subversion/svn_private_config.hc (working copy)
@@ -39,7 +39,11 @@
#define SVN_FS_WANT_DB_PATCH 14
/* Path separator for local filesystem */
+#if defined(_WIN32) || defined(_WIN64)
#define SVN_PATH_LOCAL_SEPARATOR '\\'
+#else
+#define SVN_PATH_LOCAL_SEPARATOR '/'
+#endif
/* Name of system's null device */
#define SVN_NULL_DEVICE_NAME "nul"
]]]
(Thinking of it, it should probably be enough to test for _WIN32)
> By the way, I think that Linux support of the CMake is not very
> important due to the great and useful current build system which is
> quite useful. I think it should work, but the basic target should be
> Windows or unusual platforms that autoconf doesn't have a great
> support for. However, as it is requested, I will try to improve
> support for non-Windows platforms as well...
>
Well... the autoconf build system is good and well tested but I still
believe it would be good to have one single build system for all platforms,
at least in the long run. It simplifies instructions (see the new III.F
chapter in INSTALL - we'd "only" have to provide instructions on how to get
dependencies per platform) and less dividing efforts to multiple code
bases. Maybe this discussion should be held in my thread about INSTALL,
where I floated the idea of a new unified build system.
Kind regards,
Daniel
