Dune 1.6.3 turn ocp-browser into a noop

[Drup]

with dune 1.6.2, ocp-browser works as expected
with dune 1.6.3, ocp-browser does nothing, print nothing and immediately returns.

I have no clue what's happening. I suspect it's because of jbuilder compatibility, but otherwise ...

[rgrinberg]

Hmm, it's just a bug related to how the preprocessor is being executed. I have no idea what's causing it, but the issue is gone in master.

[Drup]

Maybe it deserves a patch release? :)

I had a look. I'm still observing the issue with master. The issue is the following:

ocp-browser uses the following preprocessing action:

(preprocess  (action (with-stdout-to ${@} (run ocp-pp ${<}))))

Preprocessing actions are supposed to print the result on stdout, and dune implicitly redirects the output to the target file. This mean that the actions that are being executed for ocp-browser are in fact of the form:

(with-stdout-to file.ml.pp (with-stdout-to file.ml.pp (run ocp-pp file.ml)))

Because of the order in which we used to open and close files when executing actions, this used to have the same effect as (with-stdout-to file.ml.pp (run ocp-pp file.ml)). However, we slightly changed this in #1643 which was included in 1.6.3.

The manual documents that preprocessing actions should print the result on stdout, so technically this is a bug in ocp-browser. However, dune could have been more helpful by forbidding the use of ${@} in preprocessing actions. I grepped the dune-universe and the only other case I could find was in ocp-index. So I suggest that we update ocp-index and ocp-browser and in parallel update dune to error out on such cases.

I updated the preprocessing actions in ocp-browser and I can confirm that it works as expected with both dune 1.6.3 and master. I'm going to submit a PR.

[Drup]

Alright, that sounds good to me.

[UnixJunkie]

will there be a bugfix release?

It doesn't seem necessary. The change we pushed in dune is to forbid the use of ${@} in preprocessing actions so that users don't use it by accident. However, it wont fix the build of the last released version of ocp-browser.

OCamlPro/ocp-index#128 fixes the build of ocp-browser.

[UnixJunkie]

Thanks for the feedback. I guess it is ocp-index then that should do a bugfix release.