Skip to content
This repository was archived by the owner on Jun 20, 2019. It is now read-only.

Conversation

jpf91
Copy link
Contributor

@jpf91 jpf91 commented May 5, 2016

This pull requests rewrites most of our build Makefiles. In order to support shared libraries we will have to compile all objects once in PIC mode, once in non-PIC mode for shared libs, once in PIC mode + funittest and once in non-PIC mode + funittest. Additionally we have to link static and dynamic unittests, make sure to set the library paths correctly when running the tests...

The simplest solution is to let libtool take care of all this. Additionally it's much easier to use libbacktrace with libtool, as it already builds a .la libtool library.

This pull request therefore first adds GDC support for autoconf (autoconf.m4) and libtool (libtool.m4). gcc_support.m4 contains macros required when building in GCC (for example to set -nostdinc -I path/to/druntime for tests). Unfortunately we can extend automake in the same way. Because of that, all the hacks in d_rules.am are necessary.

As a cleanup step, the next commit splits our configure.ac code into macros in various files. As automake can't see through substituted variables we use automake conditionals for most stuff. As a bonus this means we don't specify any source files in configure. So to add new sources we only have to change the Makefile.am files.

The next commit rewrites the Makefile.am files to use libtool and automake. These Makefiles can already build & test shared libraries just fine. But without updated section support in druntime, the GC won't scan the data sections for these libraries. Nevertheless a simple hello-world links fine!
Another benefit of the new Makefiles is that we specify sources only once for unittest/library generation/header installation. So adding new sources is much simpler now.

Then there's one commit fixing issues in files we didn't compile with the old Makefiles.

The next commit switches from the old unittest.d test runner to the new test_runner.d. There's an upstream problem if all test are run in one process: Depending on compilation / module iteration order some modules might deadlock. As shared libraries and the changed source order in the new Makefiles triggers this bug, switch to the new test runner.

The test runner is a slightly extended version of the upstream test runner. Upstream simply calls the test runner once per module using the Makefile. This makes it harder to transfer the test executable
to other systems and execute it there. So we add some addditional code which iterates all compiled in modules and spawns a new process for every module test. This does essentially what the upstream Makefiles do, but this solution produces one self-contained executable.

The next commit makes shared library versioning easy: Just edit the new VERSION file in the libphobos directory. This version will be used in the soname and shared lib filename. We create libgphobos-2.067.1.so libraries instead of libgphobos.so.2.67.1. The versions after .so have a special meaning. As long as we don't have any binary compatibility we shouldn use this versioning scheme. Using libgphobos-2.067.1.so guarantees that we can install all libraries in parallel and the runtime linker will always load the correct one. (GDC will always pick the one from the libgphobos.so symlink by default)

The next commit disables some failing druntime and phobos tests. These tests did fail before, but we did not run the tests. In order to avoid delaying this pull request, I've blacklisted these tests. Once the tests are fixed, this commit should be reverted completely.

The next commit completely disables shared libraries, even if the user passes --enable-shared. Right now GDC will link to shared libs by default if available. But these libs currently don't work 100% as GC support is missing. So to avoid breaking builds with --enable-shared, disable shared libs completely.

To test shared libraries simply revert this commit and regenerate the configure/Makefile.in files.

The next commit removes support for --enable-version-specific-runtime-libs. When testing I realized this probably never worked: The gcc_version needs to be set manually in the Makefile but was never set. I first wanted to fix this, but the implementation of --enable-version-specific-runtime-libs has changed some time ago, and even with the simple fix applied, we still generate wrong paths. Is anybody even using this?

The next commit simply pushes the new generated configure/Makefile.in files.

The next commit changes GDC to always link -lgphobos -lgdruntime. Libtool does not generate a combined libgphobos.a so this is correct for shared and static libs. Although we could reproduce the old behaviour for static libs with some extra work I don't think we should have different behaviour for shared and static libs. So we ship split phobos and druntime libraries now. This way we allow users to link only against druntime if they don't need phobos. Users can build libraries against druntime and phobos or only against druntime. But if a shared phobos library includes druntime you can't link the phobos and libdruntime libraries (even indirectly via other libraries). So instead we ship a phobos only libphobos which depends on druntime. But then we also have to link applications
against libdruntime, not only phobos.

Example:

             |-libphobos
     |--libb-|-libdruntime
foo -|
     |--liba---libdruntime
     |
     |--libdruntime
     |--libphobos

The static-libphobos flags links static phobos and static druntime, without the flag shared libraries are used for both libraries. Other combinations are available with -nostdlib + Bstatic / Bdynamic.

The last commit updates the testsuite: libtool places librararies in the .libs folder. Additionally we now also need the path to libgdruntime.a.

Increased build times for the unittests are due to libtool building both -fPIC and normal objects, even though we only test normal objects. Once we support shared libraries the -fPIC objects will be used for shared library testing anyway.

Tested (before refactoring, but I think everything should still work fine):

  • Standard build + install (diffed installed directory contents to verify results)
  • Standard build + unittest
  • Out of tree build
    • native + install
    • multilib / no multilib
    • disable-shared / disable-static
    • with werror / without
    • with system libz / without
    • with libbacktrace / without
    • enable-druntime-gc / without

Still to be done:

  • Backport to GDC-5 and test all cross/canadian cross builds. I'll try to do this this weekend.

LT_INIT
AM_PROG_LIBTOOL
WITH_LOCAL_DRUNTIME([LT_LANG([D])], [])
AM_CONDITIONAL([ENABLE_SHARED], [test "$enable_shared" = "fooyes"])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fooyes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be always false for now. We don't want to build the shared libraries, even if the users uses --enable-shared. I'll change it to AM_CONDITIONAL([ENABLE_SHARED], [false]), that's more self-explaining.

jpf91 added 11 commits May 6, 2016 23:39
Old Makefiles did not compile all sources
We need to run every module test in a seperate process. Otherwise
certain module tests can mess up other modules and we might produce a
deadlock.

The test runner is a slightly extended version of the upstream test
runner. Upstream simply calls the test runner once per module using
the Makefile. This makes it harder to transfer the test executable
to other systems and execute it there. So we add some addditional
code which iterates all compiled in modules and spawns a new process
for every module test. This does essentially what the upstream
Makefiles do, but this solution produces one self-contained
executable.
These tests were not run with the old Makefile. The errors were just
hidden before.
Although these build & install just fine, there's no GC
integration yet so we don't want anybody to use this.
This is outdated and probably never worked.
(we never set gcc_version in the Makefiles)

We'd have to mirror libgcc for a updated variant.
We used to ship libgdruntime.a and libgphobos2.a where libgphobos.a
also integrated libgdruntime.a. This way we allow users to link
only agains druntime if they don't need phobos.

If we do the same thing with libphobos users can build libraries
against druntime and phobos or only against druntime. But if
a shared phobos library includes druntime you can't link the
phobos and libdruntime libraries (even indirectly via other
libraries). So instead we ship a phobos only libphobos which
depends on druntime. But then we also have to link applications
against libdruntime, not only phobos.

Example:
             |-libphobos
     |--libb-|-libdruntime
foo -|
     |--liba---libdruntime
     |
     |--libdruntime
     |--libphobos
@jpf91
Copy link
Contributor Author

jpf91 commented May 6, 2016

Updated. Also updated the description. Ping @Dicebot This might require changes to the Archlinux PKGBUILD. Feel free to criticize if you see any problem with this :-)

@mihails-strasuns
Copy link

+15,843 −2,325 :X

@ibuclaw
Copy link
Member

ibuclaw commented May 7, 2016

+15,843 −2,325 :X

Auto generated configure scripts, nothing much you can do about it.

@jpf91 did you regenerate it as per comments in configure.ac? (versions to use are still autoconf 2.64 and automake 1.11.6)

@ibuclaw
Copy link
Member

ibuclaw commented May 7, 2016

Looks good. Just two questions I'd like to ask.

  1. Any plan for something like a --disable-phobos switch (i.e: only build druntime)? Probably not worth it, as I'm not sure of the value, and it will require patching gcc build scripts.
  2. Any plan to replace separate compilation with package compilation (std/algorithm.o, std/regex.o)? Does libtool allow us to do this easily?

@mihails-strasuns
Copy link

OK, I'll have some more questions regarding shared library versioning and runtime/lib separation as soon as I get back to workdesk.

@jpf91
Copy link
Contributor Author

jpf91 commented May 7, 2016

+15,843 −2,325 :X

Auto generated configure scripts, nothing much you can do about it.

The 'regenerate autotools' commit is already +14,607 −1,626 ;-)

@jpf91 did you regenerate it as per comments in configure.ac? (versions to use are still autoconf 2.64 and automake 1.11.6)

Yes.

Any plan for something like a --disable-phobos switch (i.e: only build druntime)? Probably not worth it, as I'm not sure of the value, and it will require patching gcc build scripts.

GCC just passes unknown switches to configure, so if we do --disable-gdc-libpgohobos when configuring GCC we can check for this in our configure script. We can't use --disable-libphobos as GCC has a special rule for --disable-[folder]: In that case GCC will never call our libphobos configure script. To skip building phobos we can then simply ignore the src subdir: https://www.gnu.org/software/automake/manual/html_node/Conditional-Subdirectories.html So implementing this shouldn't be difficult, but we can do this in another pull request.

Any plan to replace separate compilation with package compilation (std/algorithm.o, std/regex.o)? Does libtool allow us to do this easily?

libtool probably does (it only needs to find the output object file, so libtool gdc a.d b.d -o c.o should work). But automake doesn't: If we use the libgphobos_la_SOURCES it will always build one object per source file.

@mihails-strasuns
Copy link

So one thing that worries me is this:

We create libgphobos-2.067.1.so libraries instead of libgphobos.so.2.67.1

Currently I have LDC with soname libphobos2-ldc.so.68 and DMD with soname libphobos2.so.0.71. That will make GDC use yet another different versioning scheme which adds to already huge pile of pain of compiler compatibility. Is there any chance to make a push for standardizing this? CC @MartinNowak

Another thing that becomes necessary once shared libraries are enabled is a way to switch between the two by using a compiler flag. See also ldc-developers/ldc#1305

@mihails-strasuns
Copy link

Ah I see, there is this:

The static-libphobos flags links static phobos and static druntime, without the flag shared libraries are used for both libraries.

It is not very convenient though, as it makes impossible to i.e. for packager to make static stdlib the default while keeping the option to switch to shared ones.

@ibuclaw
Copy link
Member

ibuclaw commented May 10, 2016

That will make GDC use yet another different versioning scheme which adds to already huge pile of pain of compiler compatibility. Is there any chance to make a push for standardizing this?

Actually, I think we should be encoding the gcc version into the library name too, because there is all likelihood that ABI's can change between major releases. So either gcc-ver.dfe-ver or gcc-ver.0.dfe-ver

  • libgphobos2.so
  • libgphobos2.so.6
  • libgphobos2.so.6.0.67 or libgphobos2.so.6.67

@ibuclaw
Copy link
Member

ibuclaw commented May 10, 2016

Or maybe we'd be ok without the gcc version...

@jpf91
Copy link
Contributor Author

jpf91 commented May 10, 2016

We create libgphobos-2.067.1.so libraries instead of libgphobos.so.2.67.1

Currently I have LDC with soname libphobos2-ldc.so.68 and DMD with soname libphobos2.so.0.71. >That will make GDC use yet another different versioning scheme which adds to already huge pile of >pain of compiler compatibility. Is there any chance to make a push for standardizing this? CC >@MartinNowak

AFAICS libphobos2.so.0.71 is wrong: http://tldp.org/HOWTO/Program-Library-HOWTO/shared-libraries.html Keeping the first digit at 0 implies that libphobos2.so.0.71 can be used instead of libphobos2.so.0.70, libphobos2.so.0.69, ... which is not true, as phobos releases are not ABI compatible!

The libphobos2-ldc.so.68 scheme correctly updates the ABI version on releases. We can probably use such versioning scheme. What happens if we introduce ABI stable phobos updates though?
We probably don't want to have libphobos.so.68, libphoobs.so.69 and libphobos.so.70 installed if libphobs.so.70 could be used for all three.

I think the best approach is:

  • Use a versioning scheme which updates the major version for every phobos release, as long as we don't have ABI compatibility (I wonder what happens with minor DMD/phobos releases though...).
  • Introduce an independent ABI versioning once we have ABI stability. E.g. Phobos 2.080 could use libphobos.so.0.0, phobos 2.081 libphobos.so.0.1, etc. When there's ABI breakage, (e.g. phobos 2.082) we increase the major number: libphobos.so.1.0.

The main problem with the LDC versioning is that it will require starting this process with a high major number, as we already 'consumed' earlier major numbers.

The static-libphobos flags links static phobos and static druntime, without the flag shared libraries are used for both libraries.

It is not very convenient though, as it makes impossible to i.e. for packager to make static stdlib >the default while keeping the option to switch to shared ones.

Shared library related compiler changes are not part of this pull request. I think we'll change GDC to use static libs be default for now and add a -shared-libphobos flag. To change the default you'll probably have to patch the sources.

Actually, I think we should be encoding the gcc version into the library name too, because there is all likelihood that ABI's can change between major releases.

EDIT:

Or maybe we'd be ok without the gcc version...

Sorry, I missed that comment before answering. Anyway:

I don't see why we should do that. The GCC backend version does not affect our ABI. Of course it's likely that compilers with different backend versions use different frontend versions, but this shouldn't affect shared library versioning. libgphobos-2.067.1.so compiled with GDC-6 should link just fine with GDC-5, so no need to encode the GCC version.

As our versioning schemes have to match what the runtime linker is expecting (See for example: http://tldp.org/HOWTO/Program-Library-HOWTO/shared-libraries.html ) we'll probably always end up with a FOO.so.MAJOR.MINOR scheme. MAJOR and MINOR have to follow some rules regarding the library ABI. Of course we can customize FOO, but different FOO names will prevent reusing ABI compatible libraries. That's why a libgphobos-VER.so.0 scheme is nice as long as we don't have ABI compatibility: We can still start at libgphobos.so.0.0 once we introduce ABI stability and we can safely install multiple versions right now.

@jpf91 jpf91 self-assigned this May 10, 2016
@jpf91
Copy link
Contributor Author

jpf91 commented May 11, 2016

OK, looks like cross compilation also works with these changes.

MinGW fails compiling at some point (2.067 broke MinGW, see ldc-developers/ldc#1121) but it seems the Makefiles work fine.

@jpf91 jpf91 removed their assignment May 12, 2016
@ibuclaw
Copy link
Member

ibuclaw commented May 16, 2016

Currently I have LDC with soname libphobos2-ldc.so.68 and DMD with soname libphobos2.so.0.71. That will make GDC use yet another different versioning scheme which adds to already huge pile of pain of compiler compatibility. Is there any chance to make a push for standardizing this? CC @MartinNowak

There is no compatibility, which is also why the library is called gphobos.

Introduce an independent ABI versioning once we have ABI stability. E.g. Phobos 2.080 could use libphobos.so.0.0, phobos 2.081 libphobos.so.0.1, etc. When there's ABI breakage, (e.g. phobos 2.082) we increase the major number: libphobos.so.1.0.

The main problem with the LDC versioning is that it will require starting this process with a high major number, as we already 'consumed' earlier major numbers.

If we were to consider being part of gcc, then we'd just bump the shared library version by one for every gcc release, regardless of what version of phobos we are using.

E.g:
libgphobos.so.1.0 - gcc-7.1
libgphobos.so.1.1 - gcc-7.2
libgphobos.so.2.0 - gcc-8.1
...

@ibuclaw
Copy link
Member

ibuclaw commented May 16, 2016

@jpf91 - Any more changes to be made here? It should be fine to delay what exact versioning to use until shared libraries are a real thing.

@jpf91
Copy link
Contributor Author

jpf91 commented May 16, 2016

Any more changes to be made here?

No, I think this is ready to go.

It should be fine to delay what exact versioning to use until shared libraries are a real thing.

Sounds like a plan 👍

@ibuclaw
Copy link
Member

ibuclaw commented May 16, 2016

OK.

@ibuclaw ibuclaw merged commit cb86a98 into D-Programming-GDC:master May 16, 2016
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.

3 participants