Skip to content
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

Drop LF < 1.14 from supported damlc output versions #11701

Merged
merged 4 commits into from
Nov 22, 2021
Merged

Conversation

cocreature
Copy link
Contributor

changelog_begin
changelog_end

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
  • If you mean to change the status of a component, please make sure you keep the Component Status page up to date.

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 cocreature force-pushed the damlc-target branch 14 times, most recently from add1ee0 to f80b8b9 Compare November 22, 2021 12:21
@cocreature cocreature force-pushed the damlc-target branch 2 times, most recently from 7743273 to f4b7abe Compare November 22, 2021 13:13
@@ -0,0 +1,237 @@
// Copyright (c) 2021 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is copied from the 1.dev version. The fact that we copy this around doesn’t seem great but fixing it also seems out of scope for this PR.

@cocreature cocreature force-pushed the damlc-target branch 4 times, most recently from dfca5b9 to 68030cf Compare November 22, 2021 13:55
fixes #11319

We keep test coverage by depending on the most recent snapshot which
still has 1.14 support.

changelog_begin

- [Daml Compiler] Damlc can only produce Daml-LF 1.14 or
  newer. Passing aynthing older to `--target` is an error. If you
  need to produce older versions, use an older SDK.

changelog_end
@cocreature cocreature marked this pull request as ready for review November 22, 2021 14:53
@cocreature cocreature requested a review from a user November 22, 2021 14:53
@@ -59,9 +59,12 @@ PROTO_LF_VERSIONS = LF_VERSIONS
# The subset of LF versions accepted by //daml-lf/encoder
ENCODER_LF_VERSIONS = ["1.dev" if ver == "dev" else ver for ver in LF_VERSIONS]

# We support older LF versions using an older compiler binary
LEGACY_COMPILER_LF_VERSIONS = ENCODER_LF_VERSIONS
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also contains 1.14 and 1.dev, should it?

Maybe we should hardcode the list of versions supported by the legacy compiler, since it presumably isn't going to keep up with new LF versions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking off this more as legacy + newer versions, i.e., everything we can build in daml_build bazel rules. Very happy for suggestions for better names.

# The subset of LF versions accepted by the compiler in the syntax
# expected by the --target option.
COMPILER_LF_VERSIONS = ENCODER_LF_VERSIONS
COMPILER_LF_VERSIONS = [ver for ver in ENCODER_LF_VERSIONS if ver not in ["1.6", "1.7", "1.8", "1.11", "1.12", "1.13"]]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The list ["1.6", "1.7", "1.8", "1.11", "1.12", "1.13"] is what I expected LEGACY_COMPILER_LF_VERSIONS to be.

If so you could rewrite this as [ver for ver in ENCODER_LF_VERSIONS if ver not in LEGACY_COMPILER_LF_VERSIONS].

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed it to that and added all_compiler_lf_versions corresponding to the previous behavior

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

changelog_begin
changelog_end
# The subset of LF versions accepted by the compiler in the syntax
# expected by the --target option.
COMPILER_LF_VERSIONS = ENCODER_LF_VERSIONS
COMPILER_LF_VERSIONS = [ver for ver in ENCODER_LF_VERSIONS if ver not in ["1.6", "1.7", "1.8", "1.11", "1.12", "1.13"]]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not using the comparator defined at line

Suggested change
COMPILER_LF_VERSIONS = [ver for ver in ENCODER_LF_VERSIONS if ver not in ["1.6", "1.7", "1.8", "1.11", "1.12", "1.13"]]
COMPILER_LF_VERSIONS = [ver for ver in ENCODER_LF_VERSIONS if _gte(ver,"1.14") ]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed the code already

@@ -1,8 +1,9 @@
-- We test for deprecations in NextMapDeprecated
{-# OPTIONS -Wno-deprecations #-}
-- Copyright (c) 2020, Digital Asset (Switzerland) GmbH and/or its affiliates.
-- All rights reserved.

-- @SINCE-LF 1.3
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not dropping this line as done in the previous file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missed that 5a3ba1e

changelog_begin
changelog_end
Copy link
Contributor

@sofiafaro-da sofiafaro-da left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks very reasonable, thanks!

@cocreature cocreature enabled auto-merge (squash) November 22, 2021 15:26
COMPILER_LF_VERSIONS = [ver for ver in ENCODER_LF_VERSIONS if ver not in ["1.6", "1.7", "1.8", "1.11", "1.12", "1.13"]]
COMPILER_LF_VERSIONS = [ver for ver in ENCODER_LF_VERSIONS if ver not in LEGACY_COMPILER_LF_VERSIONS]

ALL_COMPILER_LF_VERSIONS = LEGACY_COMPILER_LF_VERSIONS + COMPILER_LF_VERSIONS

# We need Any in DAML Script so we require DAML-LF >= 1.7
SCRIPT_LF_VERSIONS = [ver for ver in COMPILER_LF_VERSIONS if ver != "1.6"]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this definition anymore ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch, killed it b356687

changelog_begin
changelog_end
@cocreature cocreature merged commit 5785bbf into main Nov 22, 2021
@cocreature cocreature deleted the damlc-target branch November 22, 2021 16:20
azure-pipelines bot pushed a commit that referenced this pull request Nov 24, 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.

@nickchapman-da is in charge of this release.

Commit log:
```
bd2a685 Make ACS reader parameters configurable (#11732)
329e609 [Self-service error codes] Group explanations (#11715)
32e9c4b Bump go rules (#11829)
6df7af7 update compat versions for 1.18.0-snapshot.20211117.8399.0.a05a40ae (#11743)
9461702 [DPP-673][Self-service error codes] Split one narrow migration table into multiple narrower ones to fit in the generated pdf's pages. (#11803)
5519184 [DPP-673][Self-service error-codes] Generate error code directory compatible with both pdf and html docs (#11794)
5a2c070 Skip append-only migration tests on each PR (#11816)
e6c8229 port #11798 documentation changes to json-api HTTP codes documentation (#11823)
1610d97 LF: parsing interface primitives (#11825)
f2aa09c Allow encoded server key files to be in base64 [DPP-761] (#11796)
cc3f551 update NOTICES file (#11824)
1bfcbc9 Refactoring for Canton compatibility on PackageServiceErrors (#11812)
c4b6a17 [Self-service error codes] Update existing error code references [DPP-593] (#11798)
addb2ed Drop Scala 2.13 version guards from Bazel definitions (#11819)
5785bbf Drop LF < 1.14 from supported damlc output versions (#11701)
ae8391e LF Parser: handle interface (#11797)
faddba4 Increase range for excluded snapshots in compatilbility tests (#11802)
f8933d1 Make lookupTemplateChoice return only choices in the template. (#11808)
3e0ac71 update NOTICES file (#11801)
8e31e8e [Self-service error codes] Enabled by default [DPP-691] (#11685)
ab520fb Fix es (#11784)
350ad98 Bump Windows postgres (#11804)
2d60ae8 update NOTICES file (#11799)
2d9ee20 [ledger-api-client] - Do not account for deduplication_time as timeout [KVL-1172] (#11791)
f334861 LF: Simplify LF Syntax (#11795)
cbe9c92 LF: Clean up Ast (#11786)
25e5bbb kvutils - Removed unused setting from conformance tests [KVL-1174] (#11785)
cd52f13 Remove `@aherrmann-da` from `CODEOWNERS` (#11792)
9bb12d0 update NOTICES file (#11787)
d23d113 Restructure interface desugaring (#11555)
1bb2fc2  LF: Simplify transaction versionning for interface (#11744)
4b59c57 Move select DB logic to JVM [DPP-760] (#11777)
5fb5784 Limit the number of parallel Bazel actions (#11779)
a9754fe Write exception reports into the logfile in case of ssl misconfiguration (#11776)
4a8e110 Add missing export of deleteBy from DA.NonEmpty (#11716)
51f646c Add engine tests for interfaces. (#11773)
ac57355 Drop Scala 2.12 compatibility layer from //daml-lf/data (#11758)
3b5f8a7 Remove precondition field from TemplateImplements (#11763)
2e789dd [DPP-417][Self-service error codes][Docs] Add more complete information for forking gRPC status codes. (#11739)
4ebdaf5 kvutils - Deprecate deduplicate_until [kvl-1174] (#11765)
f5a6302 [participant-integration-api] - Use internal CommandCompletionService [KVL-1172] (#11741)
39a38d3 Update to Java 11 (#11512)
91b4fb9 Backport: Skip protobuf compatibility check for PRs to non-main (#11772)
a3556a5 Retry release signature checks (#11771)
8f458d8 [ledger-api] Include the completion offset in the command_service.proto responses [KVL-1171] (#11658)
e891180 participant-state: Remove the v1 API. (#11553)
869b805 Bump sandbox acquisition timeout of trigger service tests to 1m (#11764)
4106222 Fix docs regarding deduplication periods [KVL-1194] (#11738)
186ba10 Fix missing encoding of nested maps in typescript bindings (#11746)
dd88ba2 kvutils - Backwards looking command deduplication [KVL-1174] (#11634)
43438c6 sandbox: Remove PostgreSQL conformance tests. (#11434)
b0a1bf7 More missing error codes (#11749)
3366413 Delete empty scala test library (#11747)
85979b3 Upgrade to Postgres 10 (#11751)
39c5966 Drop aherrman-da from release rotation (#11756)
c8ace8b Factor kind projector target into a variable (#11759)
e78bfc7 update NOTICES file (#11757)
fa76631 Drop 2.12 versioned_scala_deps (#11748)
c77e150 [JSON-API] ContractDao JMH benchmarks against PG (#11718)
d60cf70 rotate release duty after 1.18.0-snapshot.20211116.8395.0.ccbf7140 (#11734)
bb19c0d Drop Scala 2.12 support (#11619)
c2d4ea4 Add separate commands for create/fetch/exercise by template/interface. (#11724)
7e4acf9 link to doc detailing gRPC-to-HTTP code mapping (#11742)
7974427 release 1.18.0-snapshot.20211117.8399.0.a05a40ae (#11740)
```
Changelog:
```

- [Daml Compiler] Damlc can only produce Daml-LF 1.14 or
  newer. Passing aynthing older to `--target` is an error. If you
  need to produce older versions, use an older SDK.

[Ledger API Specification] The Ledger API returns enriched error codes (see https://docs.daml.com/error-codes/self-service/index.html)
For backwards-compatibility, a new API flag `--use-pre-1.18-error-codes` is introduced for preserving the legacy behavior for
clients that want to migrate incrementally to the changed gRPC status code responses and error details format.
ledger-api-client - The default command tracking timeout is no longer influenced by the deprecated deduplication_time as a deduplication period. Previously if no timeout was being set in the command and deduplication_time was set as the deduplication period then the command tracking timeout was the minimum between the deduplication_time and max tracking timeout.
[ledger-api] - Include the completion offset in the responses from the command_service.proto
- [Integration Kit] v1 of the participant state API is no longer
  supported. You will need to upgrade to v2.

- [Typescript Bindings] Fix an issue where nested maps did not get
  encoded properly before sent to the JSON API which caused requests
  to fail with a decoding error on the JSON API.

kvutils - The deduplication duration passed in the command is now used for command deduplication, and is no longer always overwritten with the max deduplication duration. The command deduplication duration can still be extended by the committer to account for time skews.
```

CHANGELOG_BEGIN
CHANGELOG_END
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants