Add (instrumentation (libraries ...)) field

[nojb]

This PR extends the (instrumentation (backend XXX)) field to optionally take a (libraries ...) subfield:

(instrumentation (backend XXX) (libraries libA ...))

The semantics is that the libraries libA ... are ignored if the instrumentation backend is disabled, but, if enabled, they are treated as if the user had included them in the (libraries) field of the enclosing stanza.

The motivation is the recent addition of support in the landmarks instrumentation for custom backends (see https://discuss.ocaml.org/t/ann-landmarks-1-7/18319). Custom backends are implemented as external libraries that need to be linked into the final executable in order to be used. However, we would only want to link the library if the instrumentation is active, and there is no easy way to achieve that today. This feature fills that gap.

cc @maroneze who first brough up this issue in LexiFi/landmarks#57.

[Alizter]

Suggested change

	>>| List.rev
	>>| List.flatten
	>>| List.rev_concat

[nojb]

I accepted this suggestion, but to be clear, the two forms are not equivalent: rev_concat X is rev (concat X), while what I had before was concat (rev X). I don't think it makes a difference here.

[Alizter]

Actually thinking further about this, maybe its worth being correct rather than slick. Sorry for the back-and-forth.

[nojb]

Reverted in 55ed9ba

[Alizter]

I think we have

  .. versionadded:: 3.25

but I'm not sure it works here.

[nojb]

I did something in 17e813b:

[Alizter]

I think melange.emit accepts this new field but doesn't do anything with it now. You could have a go at supporting it, or otherwise user error and say its not supported at the moment.

[Alizter]

dune describe external-lib-deps also takes the --instrumentation-with flag, might be worth honoring the new field.

[Alizter]

The libraries field also accepts (select) deps. If its possible might be worth supporting, or just rejecting with a user error, up to you.

[Alizter]

Optional executables don't build when they have missing libraries. Presumably they don't account for this new field, so likely they will be tricked into being built and then fail.

[Alizter]

Another consumer of libraries maybe worth pointing out is the utop rules. Presumably something like dune utop --instrument-with ... is possible, but does nothing special at the moment.

I've also thought about ppx, but I don't have a solid grasp on how the ppx infrastructure works, so I couldn't think of any issues there.

[Alizter]

Another consumer that comes to mind that would be worth checking interaction with is the @unused-libs alias.

[nojb]

Thanks for the review @Alizter. I think I addressed all your comments. (optional) executables @unused-libs were already supported, but I added tests to confirm. @unused-libs has some limitations around -linkall but this is orthogonal to the present PR (and is documented in the test).

I think melange.emit accepts this new field but doesn't do anything with it now. You could have a go at supporting it, or otherwise user error and say its not supported at the moment.

42dbfbf

dune describe external-lib-deps also takes the --instrumentation-with flag, might be worth honoring the new field.

f92a50b

The libraries field also accepts (select) deps. If its possible might be worth supporting, or just rejecting with a user error, up to you.

681ef0f

Optional executables don't build when they have missing libraries. Presumably they don't account for this new field, so likely they will be tricked into being built and then fail.

51f9dc8

Another consumer that comes to mind that would be worth checking interaction with is the @unused-libs alias.

6296a8f

Another consumer of libraries maybe worth pointing out is the utop rules. Presumably something like dune utop --instrument-with ... is possible, but does nothing special at the moment.

b611df7