-
Notifications
You must be signed in to change notification settings - Fork 410
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
[WIP] Executables define rule for C object files (-output-obj) #23
Conversation
In ocamlbuild we used |
You can use anything from Then, so that we can reference this shared object file without hardcoding We shouldn't disable the rule, instead we should use: Build.fail { fail = fun () -> die "blah blah this is not supported" } So that we get a nice failure when requesting to build this target. |
d643151
to
223669b
Compare
I'm a little worried about the disabling of the rule, because I don't know when shared library should work or not (specially on windows). Moreover I should perhaps detect |
I'm sure fPIC is actually the right thing to look at. For instance I can see that there is a libasmrun_shared.so, which I assume is used for /cc @mshinwell who has been using it and might know more. If it's too complicated, let's just always enable the rule for now. It's only going to be a problem if you want to optionally install such a BTW there is the question of -output-obj vs -output-complete-obj. I don't know if it make sense to generate both at once. If it does maybe we could have |
I'm wondering if the rule is correct because it links the runtime and cstubs but it seems to be the goal of |
This is not correct. There are three runtimes:
I do not know what is the purpose of libasmrun_shared.so, and I do not believe the compiler will use it anywhere by itself (though you could specify it with -runtime-variant if you so wish). Any of the runtimes can be used with either As for the difference between -output-obj and -output-complete-obj, the latter links in the C stubs from autolink libraries but the former does not. Arguably -output-obj should have always done that, but I tried it and I think it was a compatibility hazard. In general, no one should use -output-obj anymore. |
@whitequark thanks for the clarifications. Let's use only
@bobot, BTW there is now a |
Thanks @whitequark for these precisions. @diml , ok I will add a test. But I again had a problem with an out of date |
@bobot I think we'll often forget to update the timestamp file. In practice, we should only need to rebuild boot.exe when the jbuild format evolves and we use the new feature in the jbuild files of jbuilder, so probably not that often. If this is a persistent problem, we could try to add |
Indeed we will need to update the timestamp file very rarely.
Yes it could be sufficient. |
ie -output-obj ocamlopt options.
ec23e22
to
f397109
Compare
This is important when it comes to cross compilation. With cross compilation I want to be able to produce an object file which would be linked to the final executable. |
@bobot, sorry i missed your last commits... I'll review this ASAP |
@diml, after the fixes have been merged to ocaml there are less problems. But this feature is really a configuration nightmare with many possible choices to provide to the user: "Should we give the possibility to link or not the stub libraries, link or not the dependencies of stub libraries, link or not the runtime, to produce shared or not library, using native or bytecode compiler" (all the choices are not given by ocaml and depend on the version). I though that we should link all the libraries that are needed for the ocaml program, ie So it is a needed feature, but perhaps we could scale back to the really needed part, production of shared libraries (principally native but byte should work too). It is what I needed and it is what #301 needs. |
Ok, yh that seems complicated indeed. Happy to scale back. So technically we just remove the Another thought: currently, if the user wants to do themselves one of the mode that is not supported, it's really painful to construct the right command line. Maybe what we could do is the following: add a (executable
(...
(custom_linkages
((mode native)
(extension ${ext_obj})
(flags (--output-obj)))))) then, if a common pattern appears we can add builtin support for it. What do you think? |
Sorry for chiming in but static libraries are useful in certain situations. For instance in my use case I can’t use sharedl ibraries because of Apple’s restrictions. They are supported - yes but it’s not allowed to use them in an App Store app. |
@wokalski would the I imagine that you would write:
it's a bit verbose, but it would unblock the situation until it's clearer what to do |
Yes, it would. I’m sorry; I missed your message. |
Ok, the doc still need to be updated but I think the code is ready. In the end we have the following: the
The following extensions are used for each linking mode:
Issueswhen compiling a I also checked the Jane Street jenga rules, and I can see that we are passing (* side-step issue where the compiler expects register r10 to be unchanged across
a call to caml_call_gc@plt, which is not true if the symbol resolution happens
right then. This works by forcing the symbol resolution to happen at load time
instead. *) |
src/exe.ml
Outdated
match m.kind with | ||
| Exe -> | ||
if mode = Native && not has_native then | ||
custom |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if the user specifies native mode explicitly, will they still get bytecode here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I'm not completely sure about this, but it seems simpler overall.
I think the pr can be merged without this, buy why is this hard to fix?
Can't we just filter the linkages for what's available and then die if that
list is empty.
…On Thu, Mar 8, 2018, 5:43 PM Jérémie Dimino ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/exe.ml
<#23 (comment)>:
> @@ -45,6 +45,52 @@ module Linkage = struct
; ext
; flags
}
+
+ let o_flags = ["-output-complete-obj"]
+ let so_flags = ["-output-complete-obj"; "-runtime-variant"; "_pic"]
+
+
+ let of_user_config (ctx : Context.t) (m : Jbuild.Executables.Link_mode.t) =
+ let has_native = Option.is_some ctx.ocamlopt in
+ let mode = m.mode in
+ match m.kind with
+ | Exe ->
+ if mode = Native && not has_native then
+ custom
Yes. I'm not completely sure about this, but it seems simpler overall.
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#23 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAIe-wJ3iqYdWemX_ckkThrV_r9QDt8Oks5tcQsSgaJpZM4Mc5gH>
.
|
Basically this is the problem: currently, if you don't put a For shared objects, the rule are not setup by default. So one needs a
Thinking about it, the 2nd solution seems best. |
BTW, @whitequark, does the following issue rings a bell: |
@rgrinberg I made the change to add Though this is a breaking change as before |
@rgrinberg I have added the doc. Could you review the last changes? |
OK, looks good. Can we have a test for a mode that isn't available if we have a good way to hide ocamlopt in a test? |
Sure, we have some tests for this in |
Should fix #22
Remains:
+PIC
? or a better error messages?