Data dependencies serialize imports

[akrmn]

This generates an $$imports value in DAML-LF with the imports needed for orphan instances when the module is included via data-dependencies

Pull Request Checklist

• Read and understand the contribution guidelines
• Include appropriate tests
• Set a descriptive title and thorough description
• Add a reference to the issue this PR will solve, if appropriate
• Include changelog additions in one or more commit message bodies between the CHANGELOG_BEGIN and CHANGELOG_END tags
• Normal production system change, include purpose of change in description

NOTE: CI is not automatically run on non-members pull-requests for security
reasons. The reviewer will have to comment with /AzurePipelines run to
trigger the build.

[cocreature]

Looks great, minor nits and we should fix the test before merging.

[cocreature]

Thanks!

[akrmn]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[akrmn]

not sure what made those tests fail, so I rebased on main to see if it helps

[cocreature]

I took a look at the failure in the damlc integration test:
It’s failing in InternedTypes. The issue is that the $$imports declaration also has a type ofc so that gets interned. Probably easiest to just modify the test to the new expected output (meaning more interned types).

[akrmn]

oh! that makes sense, thanks for looking into it @cocreature

[akrmn]

ah, the InternedTypes test also runs on older versions of the compiler. I'll add a check on the presence of $$imports and then do either the old or new check (5 interned types vs 20)

[cocreature]

ah, the InternedTypes test also runs on older versions of the compiler. I'll add a check on the presence of $$imports and then do either the old or new check (5 interned types vs 20)

I don’t think I understand that. It runs for old LF versions but only LF >= 1.11 (that’s what SINCE-LF 1.11) does.
As far as I can tell your code should apply regardless of the LF version.
Can you describe the error you’re seeing?

[akrmn]

These are the errors I got on CI:

//compiler/damlc/tests:integration-v111                                  FAILED in 169.3s
  /home/vsts/.cache/bazel/_bazel_vsts/704fb9a8c2b8a7269239cfce241f68b9/execroot/com_github_digital_asset_daml/bazel-out/k8-opt/testlogs/compiler/damlc/tests/integration-v111/test.log
//compiler/damlc/tests:integration-v112                                  FAILED in 152.4s
  /home/vsts/.cache/bazel/_bazel_vsts/704fb9a8c2b8a7269239cfce241f68b9/execroot/com_github_digital_asset_daml/bazel-out/k8-opt/testlogs/compiler/damlc/tests/integration-v112/test.log
//compiler/damlc/tests:integration-v113                                  FAILED in 165.7s
  /home/vsts/.cache/bazel/_bazel_vsts/704fb9a8c2b8a7269239cfce241f68b9/execroot/com_github_digital_asset_daml/bazel-out/k8-opt/testlogs/compiler/damlc/tests/integration-v113/test.log
//compiler/damlc/tests:integration-v16                                   FAILED in 131.2s
  /home/vsts/.cache/bazel/_bazel_vsts/704fb9a8c2b8a7269239cfce241f68b9/execroot/com_github_digital_asset_daml/bazel-out/k8-opt/testlogs/compiler/damlc/tests/integration-v16/test.log
//compiler/damlc/tests:repl                                              FAILED in 293.5s

(https://dev.azure.com/digitalasset/daml/_build/results?buildId=90136&view=logs&j=0da5d1d9-276d-5173-c4c4-9d4d4ed14fdb&t=7b720ed4-ea9c-5605-45fe-aca5cec61df4&l=9989)
I then ran //compiler/damlc/tests:integration-v111 on my machine (since I'm not sure how to fetch the log files from azure), and it failed on the same InternedTypes test, since that version didn't produce the $$imports value, but with my change the test always expects 20 interned types

[akrmn]

oh! the problem is not that older versions don't have an $$imports value, they do! but the set of orphan modules in scope is different! in particular, v111 lacks DA.Internal.Exception

[cocreature]

Oh that makes a lot more sense. exceptions only got added in LF 1.14. You could split it into two tests and limit it with @since-LF and until-lf maybe.

[akrmn]

I guess the easiest change is to weaken the length check to >= 4. The important part of this test is that the types are shared for ap1 and ap2, and the second QUERY-LF takes care of that