Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Continue with #1735 (-link-sharedlib) #2044

Closed
wants to merge 3 commits into from

Conversation

kinke
Copy link
Member

@kinke kinke commented Mar 19, 2017

Requires/includes #1960 and offers a simple cmdline switch -link-sharedlib to choose between static and shared druntime/Phobos.

The only breaking change is that -link-sharedlib defaults to true when generating a shared library via -shared and NOT using -static, so the libs specified via -defaultlib (or legacy -debuglib) will get a -shared suffix in the linker command line. Adding an explicit -link-sharedlib=false to the config file should restore full legacy behavior.

@JohanEngelen
Copy link
Member

How about -link-sharedstdlib or some other name that more clearly indicates that it only concerns druntime and Phobos?

@kinke
Copy link
Member Author

kinke commented Mar 19, 2017

-link-debuglib isn't any conciser.

@kinke
Copy link
Member Author

kinke commented Mar 19, 2017

Oh and of course there is (and will remain) another breaking change wrt. -link-debuglib without -debuglib=[...] (highly unlikely to have none of the latter as we had it in the config file). Previously, this would result in no default libs being linked against (for which you'd now use an empty -defaultlib=), whereas now a -debug suffix is appended to the -defaultlib=... names, rendering -debuglib obsolete in almost all cases.

@kinke
Copy link
Member Author

kinke commented Mar 19, 2017

Required lib filename scheme to work out of the box with a config file defining -defaultlib=<stem>: <stem>[-link-debuglib ? "-debug" : ""][-link-sharedlib ? "-shared" : ""], i.e., the one used in our runtimes CMake script (libdruntime-ldc.a, libdruntime-ldc-debug.a, libdruntime-ldc-shared.so, libdruntime-ldc-debug-shared.so).

@kinke
Copy link
Member Author

kinke commented Mar 19, 2017

This combination resolves #1282 (which in retrospective wasn't as straight-forward as David believed back then :D).

@kinke
Copy link
Member Author

kinke commented Mar 19, 2017

2 Travis jobs now hit:

Linking C shared library ../lib/libphobos2-ldc-shared.so
std/xml.o: file not recognized: File truncated
collect2: error: ld returned 1 exit status

Seems like it's trying to link it although the object is still being compiled. Wtf, make/CMake bug? Or could gcc/collect2/ld tamper with the object file and so fail if it's linked simultaneously into multiple libs (one set of Phobos objects for both static and shared libs)?

@kinke kinke added this to the 1.3.0 milestone Mar 25, 2017
@kinke
Copy link
Member Author

kinke commented Mar 25, 2017

Any concerns or do all agree this is a sane solution?

@kinke
Copy link
Member Author

kinke commented Apr 19, 2017

@klickverbot: This will need your approval or another form of feedback. ;)

Dicebot and others added 3 commits April 23, 2017 14:20
Deprecates -debuglib flag
-link-sharedlib will augment standard library names
with '-shared' suffix and -link-sharedlib=false will
cancel previously supplied -link-sharedlib
@dnadlinger
Copy link
Member

I'm fine with this. Just using libraries of the same name seemed like the prettier solution, but quite evidently didn't pan out. There is also the argument that a Shared druntime actually contains slightly different code.

The important thing to get right here is the user interface. We can later change the automatic -shared appending mechanism and -defaultlib/-debuglib flags to reading from a target configuration file (which we probably want to introduce anyway for managing multiple cross-toolchains). Do we think we might need more flavours at some point than just {static, shared} x {debug, release}?

@kinke
Copy link
Member Author

kinke commented May 3, 2017

The initial reason I went for the -shared suffix is that no suffix doesn't work on Windows when using a single libs directory (static lib phobos2-ldc.lib, import lib for phobos2-ldc.dll also phobos2-ldc.lib) and I wanted consistent default library names across platforms.

Wrt. cross-compilation and configs: we already support multiple sections in ldc2.conf based on the target triple, so e.g. copying the default section and adjusting the default switches (other libs directory, external cross-linker via -gcc=..., possibly different -defaultlib etc.) already works (used when cross-compiling from Win64 to Win32). Surely not the most elegant and non-redundant way, but very simple and powerful.

No potential additional flavours come to mind right now.

@dnadlinger
Copy link
Member

Surely not the most elegant and non-redundant way, but very simple and powerful.

Yep. What I was thinking about is a more flexible target triple matching mechanism (wildcards, …), and the ability to only overwrite part of the configuration. We could then slowly move all the hard-coded tool-chain/CPU feature/… defaults in the drive into data.

@dnadlinger
Copy link
Member

dnadlinger commented May 6, 2017

No potential additional flavours come to mind right now.

In that case, let's go with -link-sharedlib (or something more telling like -link-sharedstdlib, -stdlib-shared, … and rename -link-debuglib accordingly).

I was thinking about special builds for profiling/sanitizer/etc. purposes.

@kinke
Copy link
Member Author

kinke commented May 6, 2017

We could also add an additional -stdlib-shared-suffix option defaulting to -shared but overridable by distros only shipping with shared libs (to an empty suffix in their config file to disable the effect of -stdlib-shared or whatever).

@dnadlinger
Copy link
Member

dnadlinger commented May 6, 2017

We could also add an additional -stdlib-shared-suffix option

I'd rather add support for build type matching to the config file, and then have just a simple library name setting. Something like (pseudocode):

match:
  stdlib_debug: false
  match:
    link_shared: true
    stdlib_library_names: ["druntime-ldc-shared", "phobos2-ldc-shared"]

This would avoid having to add more flags for potentially different system libraries dependencies and so on, while offering a lot of flexibility for weird cross-compilation setups (e.g. simulator builds) and so on.

@kinke kinke removed this from the 1.3.0 milestone May 16, 2017
@dnadlinger
Copy link
Member

We should resurrect this, or something along the lines. Users having to manually type out the defaultlib/debuglib switches is getting embarrassing, especially with shared library support now being mature. I still don't like the idea of -stdlib-shared-suffix, but for now we can just hardcode the -shared suffix and add more flexible configuration options later.

@kinke
Copy link
Member Author

kinke commented Dec 10, 2017

Fine by me, but we should think carefully about how to name that switch. I guess there's enough confusion about the terms 'runtime library' (druntime only?), 'standard library' (Phobos only?) and 'default library' already (mea culpa too). Wrt. the existing -defaultlib=druntime-ldc,phobos2-ldc, I'm nowadays inclined towards naming the 2 switches -defaultlib-{debug,shared}, what do you think?

@kinke
Copy link
Member Author

kinke commented Dec 17, 2017

Superseded once more, by #2443 this time.

@kinke kinke closed this Dec 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants