Data dependencies deserialize imports #11037
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cocreature
reviewed
Sep 28, 2021
There was a problem hiding this comment.
Nice! I would suggest to merge this after #11038 and add an integration test to https://github.com/digital-asset/daml/blob/main/compiler/damlc/tests/src/DA/Test/DataDependencies.hs in this PR.
| @@ -199,6 +211,9 @@ modRefImport Config{..} ModRef{..} = noLoc ImportDecl | |||
| | importPkgId == configSelfPkgId -> modRefModule | |||
| -- The module names from the current package are the only ones that are not modified | |||
| | otherwise -> prefixDependencyModule importPkgId modRefModule | |||
| impSpec = case modRefImpSpec of | |||
| NoImpSpec -> Nothing | |||
| EmptyImpSpec -> Just (False, noLoc []) -- False = not 'hiding' | |||
There was a problem hiding this comment.
Types are amazing!
-- GHC devs
Bool
-- also GHC devs
| Just LF.DefValue{dvalBinder=(_, ty)} <- [NM.lookup LFC.moduleImportsName . LF.moduleValues $ envMod env] | ||
| Just quals <- [LFC.decodeModuleImports ty] | ||
| LF.Qualified { LF.qualModule, LF.qualPackage } <- Set.toList quals | ||
| pure $ emitModRef ModRef |
There was a problem hiding this comment.
What happens if we have both a reference to a type in a module as well as an orphan instance? It looks like we want a semigroup instance on ModRefImpSpec where NoImpSpec takes precedence and then change emitModRef to make sure that it overwrites.
There was a problem hiding this comment.
Hm, I think the only way to test that would involve solving #10772 first, since otherwise there's no way (that I can think of) for "new-package" Main.daml to import the instance and a type defined in module "old-package" X.daml without importing X.daml directly.
There was a problem hiding this comment.
in any case, I think this would only trigger a redundant import warning, but I'd have to check
6 tasks
akrmn
force-pushed
the
data-dependencies-imports-encoding
branch
from
93fa3fc to
927cb62
Compare
akrmn
force-pushed
the
data-dependencies-deserialize-imports
branch
from
ddfc0db to
b2f53f0
Compare
akrmn
changed the base branch from
main
to
data-dependencies-serialize-imports
akrmn
commented
Sep 29, 2021
akrmn
commented
Sep 29, 2021
cocreature
approved these changes
Sep 29, 2021
akrmn
force-pushed
the
data-dependencies-serialize-imports
branch
from
0f87a36 to
92bb3da
Compare
akrmn
force-pushed
the
data-dependencies-deserialize-imports
branch
from
b2f53f0 to
bf9e8e1
Compare
akrmn
force-pushed
the
data-dependencies-serialize-imports
branch
from
92bb3da to
673b55c
Compare
akrmn
force-pushed
the
data-dependencies-deserialize-imports
branch
from
bf9e8e1 to
2bbcb1f
Compare
akrmn
force-pushed
the
data-dependencies-serialize-imports
branch
from
673b55c to
9f624a9
Compare
akrmn
force-pushed
the
data-dependencies-deserialize-imports
branch
from
2bbcb1f to
aba250b
Compare
akrmn
force-pushed
the
data-dependencies-serialize-imports
branch
from
9f624a9 to
fe96730
Compare
akrmn
force-pushed
the
data-dependencies-deserialize-imports
branch
from
aba250b to
d0129e0
Compare
Base automatically changed from
data-dependencies-serialize-imports
to
main
September 30, 2021 15:25
…orts' changelog_begin changelog_end
… a multiple-module library and app
akrmn
force-pushed
the
data-dependencies-deserialize-imports
branch
from
d0129e0 to
480c1a0
Compare
azure-pipelines bot
pushed a commit
that referenced
this pull request
Oct 6, 2021
This PR has been created by a script, which is not very smart and does not have all the context. Please do double-check that the version prefix is correct before merging. @adriaanm-da is in charge of this release. Commit log: ``` cfcdc13 Add deprecation output for sandbox classic (#11119) ac5c52d DPP-609 Add StorageBackendTestsDeduplication (#11088) 55af7ad HttpServiceTestFixture provides a sandbox-classic ledger used for performance tests (#11128) 63ab3f3 Fix change ID references in services.rst (#11130) 31db15d Port error code changes (#11113) 9fd8182 move reusable functions from ContractsFetch to new fetch-contracts library (#11057) c2084f6 Try and improve the explanation of compatibility test exclusions (#11114) 4df9b7c Rename command completion dedup time to duration in participant code [KVL-1057] (#10901) 11bc22d LF: builtins to create, signatory, and obersver on interface payload. (#11120) d4cb1f9 KV ledgers: disable participant side command de-duplication [KVL-1083] (#11095) b9a5a83 Minor: fix trace logging in `TimeBasedWriteSetSelector` (#11117) 018e908 [JSON-API] Make key_hash idx non unique (#11102) 429f437 Remove HA indexer integration test on H2 (#11104) 9cffa1f LF: check LF transaction protobuf aginst local snapshots (#11064) d7ee278 Optional table prefix for trigger service (#11047) 268b2d3 Command deduplication - test case that uses CommandService and CommandSubmissionService [KVL-1106] (#11098) fac05f6 bypass value enrichment for the performance test (#11112) 4860271 Port file was not written to... where? (#11108) a8d703b Increase akka server request-timeout for HttpServiceIntegrationTests (#11109) 37d94aa Add unit for daily performance tests (#11107) eb4ac8a Increase timeouts for resetting database (#11103) db75f7d interface methods: Scala Typechecker (#11097) d3e6f16 [JSON-API] Migrating tests to use sandbox next (#11034) ffc8d68 Data dependencies deserialize imports (#11037) 4b31bf0 update compat versions for 1.17.0 (#11075) 9fd6326 interfaces: consuming/non-consuming iface choices (#11009) 229ce47 Data dependencies serialize imports (#11038) 5d9ec65 Add test case for parallel command deduplication using mixed clients [KVL-1090] (#11093) d64d965 kvutils: Print state updates before and after normalization. (#11096) be216aa update NOTICES file (#11089) 517e866 Extract common code for command dedup conformance tests [KVL-1090] (#11092) 7a4963b add `--max-inbound-message-size` flag to `daml ledger export` (#11087) 26d10b2 Drop NodeId type parameter (#10921) 3cbd922 More NonEmpty functions (#10930) bcd4686 interface methods: Speedy (#11076) 85adaab [DPP-417][DPP-595] Error codes switching - follow-up (#11074) d7b280b Remove debug print in daml-lf/interpreter build (#11091) 1ed6428 LF: move archive snapshots in a separate directory (#11081) 5e424f8 Command dedup conformance suites readme (#11051) 8290347 DPP-496 HA indexer integration test (#11033) 735c309 Switch from InputStream to Byte Array at the binary content JDBC transport (#11072) 6ae3afa fix perf reporting (#11073) bf801a6 Fix SimpleDeduplicationBasic when participant deduplication is disabled (#11083) 62234dc [Short] Remove unused code (#11079) c1d1521 interface methods: Scala AST (#11070) 7dd9c2d Remove expectations for internal failures from parallel command dedup tests (#11061) 5112599 Explicit discard in daml-lf/interpreter (#11067) 554b36c rotate release duty after 1.18.0-snapshot.20210928.7948.0.b4d00317 (#11059) d156964 Release SDK 1.17.0 (#11062) 6d8cf70 Handle interface methods in LF conversion (#11054) 87ecf1f daml-ledger: better errors for non-json responses. (#11055) ec2d26f [Divulgence pruning] Fixes divulgence pruning offset update query (#11046) f13c6d6 release 1.18.0-snapshot.20210928.7948.0.b4d00317 (#11058) d1805a3 More dependabot fun (#11063) 07b273a update NOTICES file (#11060) ``` Changelog: ``` - [JSON-API] make key_hash indexes non-unique, this fixes a bug where a duplicate key conflict was raised on the query store when the same contract was being witnessed twice by two separate parties * [Trigger Service] Enable the new `tablePrefix` setting in the `--jdbc` flag to add a prefix to all tables used by the trigger service to avoid collisions with other components using the same db-schema. * [Trigger Service] Enable the new ``--allow-existing-schema`` flag to initialize the trigger service on a database with a pre-existing schema. - [Daml Assistant] Add support for `--max-inbound-message-size` flag to the Ledger Export tool. Add cons, find, delete and deleteBy functions to NonEmpty module. ``` CHANGELOG_BEGIN CHANGELOG_END
adriaanm-da
pushed a commit
that referenced
this pull request
Oct 6, 2021
This PR has been created by a script, which is not very smart and does not have all the context. Please do double-check that the version prefix is correct before merging. @adriaanm-da is in charge of this release. Commit log: ``` cfcdc13 Add deprecation output for sandbox classic (#11119) ac5c52d DPP-609 Add StorageBackendTestsDeduplication (#11088) 55af7ad HttpServiceTestFixture provides a sandbox-classic ledger used for performance tests (#11128) 63ab3f3 Fix change ID references in services.rst (#11130) 31db15d Port error code changes (#11113) 9fd8182 move reusable functions from ContractsFetch to new fetch-contracts library (#11057) c2084f6 Try and improve the explanation of compatibility test exclusions (#11114) 4df9b7c Rename command completion dedup time to duration in participant code [KVL-1057] (#10901) 11bc22d LF: builtins to create, signatory, and obersver on interface payload. (#11120) d4cb1f9 KV ledgers: disable participant side command de-duplication [KVL-1083] (#11095) b9a5a83 Minor: fix trace logging in `TimeBasedWriteSetSelector` (#11117) 018e908 [JSON-API] Make key_hash idx non unique (#11102) 429f437 Remove HA indexer integration test on H2 (#11104) 9cffa1f LF: check LF transaction protobuf aginst local snapshots (#11064) d7ee278 Optional table prefix for trigger service (#11047) 268b2d3 Command deduplication - test case that uses CommandService and CommandSubmissionService [KVL-1106] (#11098) fac05f6 bypass value enrichment for the performance test (#11112) 4860271 Port file was not written to... where? (#11108) a8d703b Increase akka server request-timeout for HttpServiceIntegrationTests (#11109) 37d94aa Add unit for daily performance tests (#11107) eb4ac8a Increase timeouts for resetting database (#11103) db75f7d interface methods: Scala Typechecker (#11097) d3e6f16 [JSON-API] Migrating tests to use sandbox next (#11034) ffc8d68 Data dependencies deserialize imports (#11037) 4b31bf0 update compat versions for 1.17.0 (#11075) 9fd6326 interfaces: consuming/non-consuming iface choices (#11009) 229ce47 Data dependencies serialize imports (#11038) 5d9ec65 Add test case for parallel command deduplication using mixed clients [KVL-1090] (#11093) d64d965 kvutils: Print state updates before and after normalization. (#11096) be216aa update NOTICES file (#11089) 517e866 Extract common code for command dedup conformance tests [KVL-1090] (#11092) 7a4963b add `--max-inbound-message-size` flag to `daml ledger export` (#11087) 26d10b2 Drop NodeId type parameter (#10921) 3cbd922 More NonEmpty functions (#10930) bcd4686 interface methods: Speedy (#11076) 85adaab [DPP-417][DPP-595] Error codes switching - follow-up (#11074) d7b280b Remove debug print in daml-lf/interpreter build (#11091) 1ed6428 LF: move archive snapshots in a separate directory (#11081) 5e424f8 Command dedup conformance suites readme (#11051) 8290347 DPP-496 HA indexer integration test (#11033) 735c309 Switch from InputStream to Byte Array at the binary content JDBC transport (#11072) 6ae3afa fix perf reporting (#11073) bf801a6 Fix SimpleDeduplicationBasic when participant deduplication is disabled (#11083) 62234dc [Short] Remove unused code (#11079) c1d1521 interface methods: Scala AST (#11070) 7dd9c2d Remove expectations for internal failures from parallel command dedup tests (#11061) 5112599 Explicit discard in daml-lf/interpreter (#11067) 554b36c rotate release duty after 1.18.0-snapshot.20210928.7948.0.b4d00317 (#11059) d156964 Release SDK 1.17.0 (#11062) 6d8cf70 Handle interface methods in LF conversion (#11054) 87ecf1f daml-ledger: better errors for non-json responses. (#11055) ec2d26f [Divulgence pruning] Fixes divulgence pruning offset update query (#11046) f13c6d6 release 1.18.0-snapshot.20210928.7948.0.b4d00317 (#11058) d1805a3 More dependabot fun (#11063) 07b273a update NOTICES file (#11060) ``` Changelog: ``` - [JSON-API] make key_hash indexes non-unique, this fixes a bug where a duplicate key conflict was raised on the query store when the same contract was being witnessed twice by two separate parties * [Trigger Service] Enable the new `tablePrefix` setting in the `--jdbc` flag to add a prefix to all tables used by the trigger service to avoid collisions with other components using the same db-schema. * [Trigger Service] Enable the new ``--allow-existing-schema`` flag to initialize the trigger service on a database with a pre-existing schema. - [Daml Assistant] Add support for `--max-inbound-message-size` flag to the Ledger Export tool. Add cons, find, delete and deleteBy functions to NonEmpty module. ``` CHANGELOG_BEGIN CHANGELOG_END Co-authored-by: Azure Pipelines DAML Build <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This reads the value of the
$$importsvalue in the previously generated DAML-LF and produces the corresponding imports withemitModRefPull Request Checklist
CHANGELOG_BEGINandCHANGELOG_ENDtagsNOTE: CI is not automatically run on non-members pull-requests for security
reasons. The reviewer will have to comment with
/AzurePipelines runtotrigger the build.