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
ocamlbuild does not observe OCAMLPARAM #6999
Comments
Comment author: @gasche Meh: one more reason why OCAMLPARAM, while offering enticing convenience to the user in a hurry, is a pain to handle well. But this needs to be fixed nonetheless. |
Comment author: @gasche So there are many possible approaches to fix this issue, and I would Basically the problem is that the current way OCamlbuild decides The current code taking this rerun-or-not decision can be found in the The information taken into acount for the hit-or-miss decision is: the Environment parameters, as/if they do not appear in the final command, Here are some ways to fix this:
I feel that (1) is too specific. (3) is probably the least invasive |
Comment author: @whitequark The fact that the contents or even location of findlib.conf is not recorded has really made my day worse a lot of times with opam-android. (1) is awful. I do not have a strong preference between (2) and (3), there are pros and cons to both. I think one reason to go (3) is that typically, *nix commands look at all the same things in the environment, not different things based on the rule (invocation). |
Comment author: @gasche
I don't understand your point. My idea was to encode/store this extra state in the Command.t value returned by the rule's action. This would not be done implicitly, so the user would write something like, instead of
something like Cmd (S [!Options.ocamlc; flag; T tags; where ocamlc_env would be a list of environment variable names that would be turned into (name,value) pairs at the time where the command is turned into a real runnable command. In any case, the code of the rule action (or of the auxiliary functions called by the rule action, etc.) would need to be modified to take this side-information into account. So I don't understand why this better reflect the model where "*nix commands look at all the same things", because we would have to modify every command in our actions to support that (we can factorize the modification under a common name, say for all ocaml-related actions, but that seems equally true of (2)). (Let's clarify now that I am not suggesting to take a snapshot of the whole user environment, as this changes much too often. I don't think anyone believed that -- but it means we should be explicit about which part of the environment to track, potentially in all rules.) |
Comment author: @gasche I think giving a concrete code example idea above may be useful to drive the discussion -- it's more explicit. For (2), I was thinking of turning a rule of the form (fun env build -> into something like (assuming we add an optional parameter as a broken way to hope to remain backward-compatible): (fun env build ~dyn_input -> which would record whatever dyn_input is called with as a "dynamic side-input of the rule", much like whatever is passed to "build" is recorded as a dynamic dependency. (A monadic API would be nicer, because we could have a small sub-library for dynamic inputs that would pack "compute the dynamic inputs" and "record them" into a single bind action, but that's a much more invasive change for another day.) Again, that requires modification of all rules, possibly in an uglier way -- but one may claim that hacking the Command datatype into a side-channel container also distorts our API in unpleasant ways. |
Comment author: @whitequark Oh, I see your point now. I can say I do not have any preference between those at all, not the least because I lack any high-level 'architectural vision' of ocamlbuild, I just hack things together using it. I would trust your judgement here. |
Comment author: @damiendoligez ocamlbuild is now a separate project that lives on GitHub. |
Original bug ID: 6999
Reporter: @whitequark
Status: resolved (set by @damiendoligez on 2017-02-24T10:49:26Z)
Resolution: suspended
Priority: normal
Severity: minor
Target version: later
Category: -for ocamlbuild use https://github.com/ocaml/ocamlbuild/issues
Bug description
This will lead to incorrect results if the value of OCAMLPARAM changes between runs, as ocamlbuild will use cached build products.
The text was updated successfully, but these errors were encountered: