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

Tries to build cmo files for native target(s) #4613

Closed
vicuna opened this issue Sep 14, 2008 · 6 comments
Closed

Tries to build cmo files for native target(s) #4613

vicuna opened this issue Sep 14, 2008 · 6 comments

Comments

@vicuna
Copy link

vicuna commented Sep 14, 2008

Original bug ID: 4613
Reporter: jessicah
Assigned to: @gasche
Status: closed (set by @xavierleroy on 2015-12-11T18:19:51Z)
Resolution: duplicate
Priority: normal
Severity: minor
Version: 3.11+dev
Target version: 4.02.0+dev
Category: -for ocamlbuild use https://github.com/ocaml/ocamlbuild/issues
Has duplicate: #5282
Related to: #4421
Monitored by: @bobzhang @ygrek @glondu jessicah @jberdine @hcarty

Bug description

Invoking: ocamlbuild snowflake/asm.cmx, it tries to build snowflake/asm.cmo

I have also seen this issue in 3.10.0 & 3.10.2.

From what I can tell, ocamlbuild forces building of cmo when building cmi if no mli file is present.

Not only is it unnecessary work, ocamlbuild appears to throw away the cmo at some point, and rebuilds the cmo everytime ocamlbuild is run on a target that invokes this behaviour.

Additional information

Part of the output from ocamlbuild with -verbose 10

Doing sanity checks
include directories are: [ "." ]
==> kernel/asm.cmx
====> kernel/asm.mlpack
====> kernel/asm.ml
kernel/asm.ml exists and up to date
====> kernel/asm.ml.depends
======> kernel/asm.ml
kernel/asm.ml already built
start rule ocaml dependencies ml (%=kernel/asm )
....
new dyndep for "ocaml: ml -> cmo & cmi (%=kernel/asm )"([ kernel/asm.cmo;
kernel/asm.cmi ]): "libraries/stdlib/pervasives.cmi"

File attachments

@vicuna
Copy link
Author

vicuna commented Sep 14, 2008

Comment author: jessicah

Attached a patch to avoid use of ocamlc in native-only projects.

Also tested building ocaml source tree with patch applied using build/fastworld.sh without error.

This should also resolve issue 0004421.

Might also be better to make condition more explicit, e.g. (Tags.mem "native" tags) and not (Tags.mem "byte" tags).

@vicuna
Copy link
Author

vicuna commented Mar 4, 2010

Comment author: @ygrek

Wouldn't it be sufficient to add native counterpart for the following rule (the only one that produces cmi currently, hence the problem):

rule "ocaml: ml -> cmo & cmi"
~tags:["ocaml"]
~prods:["%.cmo"; "%.cmi"]
~deps:["%.ml"; "%.ml.depends"]
(Ocaml_compiler.byte_compile_ocaml_implem "%.ml" "%.cmo");;

i.e.

rule "ocaml: ml -> cmx & o & cmi"
~tags:["ocaml"; "native"]
~prods:["%.cmx"; x_o; "%.cmi"]
~deps:["%.ml"; "%.ml.depends"]
(Ocaml_compiler.native_compile_ocaml_implem "%.ml");;

And add "%.mli" to the ~deps of rule "ocaml: ml & cmi -> cmx & o" (for the same purpose it is done for byte rules, to handle the situation when mli is provided).

Currently default rules suggest that cmi is required to build cmx, but this is not true, isn't it?

@vicuna
Copy link
Author

vicuna commented May 1, 2010

Comment author: jessicah

That doesn't work. There is also the rule "ocaml: mli -> cmi" that'll trigger ocamlc over ocamlopt as well.

Given in normal circumstances, it doesn't affect anybody, might as well close issue.

@vicuna
Copy link
Author

vicuna commented Jun 16, 2013

Comment author: @gasche

I just integrated in trunk the remaining bit of snowflake.patch that was not already in. Compiling .cmi from .mli now uses ocamlopt when the tag "native" is set.

If I understand the issue correctly, that was the last issue remaining for ygrek's proposal to be a satisfying solution to the original problem. However, I do not understand the reasons why the present implementation takes care to force using the ".cmi & .cmo" rule (as explicitly said in comments: "no cmi here you must make the byte version to have it"), so I don't want to rush implementing this.

ygrek, can you comment on why do you think the current implementation is as it is, and do you have confidence that your proposed change has no negative side-effects? Otherwise I'll try to ping Nicolas on this.

In any case, I relegate the remaining work to 4.02+dev.

@vicuna
Copy link
Author

vicuna commented Jun 16, 2013

Comment author: @ygrek

Sorry, no, I don't have enough expertise here, my proposal was based on a superficial glance over source.

@vicuna
Copy link
Author

vicuna commented Jun 17, 2013

Comment author: @gasche

As the problem of the .cmo being built is also the topic of #5282, I will close this one to facilitate bugtracking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants