Skip to content
This repository has been archived by the owner on Aug 2, 2020. It is now read-only.

Fix dynamic way #4

Closed
snowleopard opened this issue Dec 20, 2015 · 18 comments
Closed

Fix dynamic way #4

snowleopard opened this issue Dec 20, 2015 · 18 comments
Labels
Milestone

Comments

@snowleopard
Copy link
Owner

Dynamic way is currently broken (another relevant issue: #3).

Most infrastructure is already there, but one needs to identify the missing bits, find the relevant parts in the the old build system, and translate them into the new build system.

Feel free to pick this up if you'd like to contribute.

@bgamari
Copy link
Collaborator

bgamari commented Dec 20, 2015

It looks like there are a few things that we might lack,

  • rules/build-package.mk has an conditional on BuildSharedLibs which seems to provide rules for building .c and .s files for the dyn way. However, the existing rule in compilePackage might already cover this.
  • rules/build-package-way.mk provides a rule that looks something like this on Windows,
$$($1_$2_$3_LIB) : $$($1_$2_$3_ALL_OBJS) $$(ALL_RTS_LIBS) $$($1_$2_$3_DEPS_LIBS)
ifneq "$$($1_$2_$3_LIB0)" ""
    $$(call build-dll,$1,$2,$3,-L$1/$2/build -l$$($1_$2_$3_LIB0_ROOT),$$(filter-out $$($1_$2_dll0_HS_OBJS),$$($1_$2_$3_HS_OBJS)) $$($1_$2_$3_NON_HS_OBJS),$$@)
else
    $$(call build-dll,$1,$2,$3,,$$($1_$2_$3_HS_OBJS) $$($1_$2_$3_NON_HS_OBJS),$$@)
endif

ifneq "$$($1_$2_$3_LIB0)" ""
$$($1_$2_$3_LIB) : $$($1_$2_$3_LIB0)
$$($1_$2_$3_LIB0) : $$($1_$2_$3_ALL_OBJS) $$(ALL_RTS_LIBS) $$($1_$2_$3_DEPS_LIBS)
    $$(call build-dll,$1,$2,$3,,$$($1_$2_dll0_HS_OBJS) $$($1_$2_$3_NON_HS_OBJS),$$($1_$2_$3_LIB0))
endif

and like this otherwise,

$$($1_$2_$3_LIB) : $$($1_$2_$3_ALL_OBJS) $$(ALL_RTS_LIBS) $$($1_$2_$3_DEPS_LIBS)
    $$(call cmd,$1_$2_HC) $$($1_$2_$3_ALL_HC_OPTS) $$($1_$2_$3_GHC_LD_OPTS) $$($1_$2_$3_ALL_OBJS) \
         -shared -dynamic -dynload deploy \
     $$(addprefix -l,$$($1_$2_EXTRA_LIBRARIES)) $$(addprefix -L,$$($1_$2_EXTRA_LIBDIRS)) \
         -no-auto-link-packages \
         -o $$@

@snowleopard
Copy link
Owner Author

I am copying an issue reported by @kgardas (#198) here:

I've not seen issue for this, but both my Solaris and OpenBSD builds end with missing shared libs:

bindisttest$ ../inplace/bin/ghc-stage2 -dynamic --make HelloWorld.lhs 
[1 of 1] Compiling Main             ( HelloWorld.lhs, HelloWorld.o )
HelloWorld.lhs:3:8: error:
    Failed to load interface for ‘Prelude’
    Perhaps you haven't installed the "dyn" libraries for package ‘base-4.9.0.0’?
    Use -v to see a list of the files searched for.

probably this is general issue and we do not build shared libs yet? If it's already noted somewhere please feel free to close it. Thanks!

@izgzhen
Copy link
Collaborator

izgzhen commented Apr 18, 2017

Just curious: It seems that there was no previous code on this issue, so I wonder if hadrian is ever shipped with this functionality before (since "fixing" .... I mean).

Also, I can't find the buildPackage as described in paper.

@snowleopard
Copy link
Owner Author

@izgzhen The dynamic way was never implemented. The "fixing" refers to it being broken :-)

This is the equivalent of buildPackage from the paper:

https://github.com/snowleopard/hadrian/blob/master/src/Rules.hs#L46-L72

(Apologies for the slow responses, I'm currently travelling.)

@snowleopard
Copy link
Owner Author

@izgzhen By the way, if you are interested in taking the lead on this issue, it'd be great to first talk to Jose Calderon (@jmct).

@izgzhen
Copy link
Collaborator

izgzhen commented Apr 26, 2017

@snowleopard @jmct Yes, I am interested in looking into that issue. I have tried to capture the need for dynamic libs (when dyn is on) with this inserted in packageRules :

buildDynLib :: Rules ()
buildDynLib =
  "_build//*.so" %> so -> error $ show so

However, I don't quite get where does the need for these shared libs come from. I can't find such a rule in Hadrian that inspects the way flag and then need for certain dynamic libraries to be built.

@snowleopard
Copy link
Owner Author

@izgzhen The top-level need for all libraries is here:

https://github.com/snowleopard/hadrian/blob/master/src/Rules.hs#L27-L44

The path to the library file is computed by calling the pkgLibraryFile function:

https://github.com/snowleopard/hadrian/blob/master/src/Settings/Path.hs#L78-L83

I think this code should do what you want, but you need to enable the dynamic way, by uncommenting these lines:

https://github.com/snowleopard/hadrian/blob/master/src/Settings/Default.hs#L156-L157

Let me know if this works for you!

@jmct
Copy link

jmct commented Apr 26, 2017

Hey @izgzhen, it’s awesome that you want to help out on this!

I’m not sure of your schedule but perhaps we can get on chat and coordinate a plan sometime soon. Work has meant that I haven’t been able to tackle this issue completely but I do have some work in progress.

@izgzhen
Copy link
Collaborator

izgzhen commented Apr 27, 2017

@snowleopard Yes, this is exactly what I am finding! Thanks a lot

@jmct Great, what is the status of your work?

@izgzhen
Copy link
Collaborator

izgzhen commented Jun 11, 2017

I need to get the DynamicExtension setting to write the target for the its Rules, but it seems like I can't run an Action inside Rules to get the returned value.

@snowleopard @ndmitchell

@snowleopard
Copy link
Owner Author

snowleopard commented Jun 11, 2017

@izgzhen Yes, I expected this could be a problem.

A pragmatic solution is to create a build rule that matches a predefined set of extensions (dll etc.), but need only the right one.

P.S.: In fact, each dynamic extension will probably have a different build rule anyway. So, all we need is to need the right file, e.g. here: https://github.com/snowleopard/hadrian/blob/master/src/Rules.hs#L28

@izgzhen
Copy link
Collaborator

izgzhen commented Jun 12, 2017

Well, just found that we need setting ProjectVersion in target path as well, which is basically impossible to be predefined! A workaround is to not need for the precisely named one, but just need something like libHSrts.dylib first, then adding an action copying it to libHSrts-ghc8.3.20170608.dylib by the end of build. But we can't track this.

I still don't get why we can't use these information in Rules (), since they are already there right?

@izgzhen
Copy link
Collaborator

izgzhen commented Jun 12, 2017

Oops -- maybe I can just relax the match pattern

@snowleopard
Copy link
Owner Author

@izgzhen I don't understand. Why can't you add need ["libHSrts-ghc8.3.20170608.dylib"] to top-level targets (as I linked) and add //*.dynlib build rule?

@izgzhen
Copy link
Collaborator

izgzhen commented Jul 4, 2017

Is now the time to close this since I opened another issue #248?

@snowleopard
Copy link
Owner Author

@izgzhen What about dynamic way on Windows? We could perhaps open a new Windows-specific issue.

@izgzhen
Copy link
Collaborator

izgzhen commented Jul 4, 2017

What about dynamic way on Windows?

Oh, right. I almost forget this. I think it is a good idea, since I might not be able to track this platform closely in near term, so a fresh place for us to discuss the specifics sounds appealing.

@snowleopard
Copy link
Owner Author

Sounds good. I'll create a new issue.

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants