SG19-5: enforce package import table #21847

roger-bosman-da · 2025-09-03T09:48:14Z

Initial logic, with "debugging packages" ifDebug Move to set, fix

Q

Initial logic, with "debugging packages" ifDebug Move to set, fix

roger-bosman-da · 2025-09-08T11:47:06Z

/azp run

azure-pipelines · 2025-09-08T11:47:12Z

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

lg Add imports to EncodeModule rule, fixing tests?🙏

roger-bosman-da · 2025-09-12T15:11:09Z

sdk/compiler/daml-lf-ast/src/DA/Daml/LF/Ast/Util.hs

+stable packages will only depend on existing stable packages, adding its hash to
+this list won't change the hash.
+-}
+stableIds :: [PackageId]


Possibly move to some bazel resource

roger-bosman-da · 2025-09-12T15:20:23Z

sdk/daml-lf/archive/src/main/scala/com/digitalasset/daml/lf/archive/DecodeV2.scala

      .withDefault(_ => throw Error.Parsing("BuiltinFunction.UNRECOGNIZED"))
+
+  // need to put at central place
+  val stableIds: Seq[PackageId] =


Perhaps to some bazel resource

roger-bosman-da · 2025-09-12T15:20:35Z

sdk/daml-lf/encoder/src/main/scala/com/digitalasset/daml/lf/archive/testing/EncodeV2.scala

      }

+    // need to put at central place
+    val stableIds: Seq[PackageId] =


perhaps to some bazel resource

remyhaemmerle-da

first comments.

remyhaemmerle-da · 2025-09-18T11:17:26Z

sdk/daml-lf/archive/src/main/protobuf/com/digitalasset/daml/lf/archive/daml_lf2.proto

+    string no_imported_packages_reason = 8;
+    PackageImports package_imports = 9;


Suggested change

string no_imported_packages_reason = 8;

PackageImports package_imports = 9;

string no_imported_packages_reason = 8; // *Available in versions >= 2.dev*

PackageImports package_imports = 9; // *Available in versions >= 2.dev*

remyhaemmerle-da · 2025-09-18T11:21:58Z

sdk/daml-lf/archive/src/main/scala/com/digitalasset/daml/lf/archive/DecodeV2.scala

+  private[archive] def decodePackageImports(
+      lfPackage: PLF.Package
+  ): Either[String, collection.IndexedSeq[String]] = {
+    lfPackage.getImportsSumCase match {


we need to reject if the field is set and LF = 2.1.
I think we need to reject if the field is unset and LF 2.dev

paulbrauner-da

Just a first round of comments, will review the bulk of the PR tomorrow.

paulbrauner-da · 2025-09-18T17:26:20Z

sdk/compiler/daml-lf-ast/src/DA/Daml/LF/Ast/Base.hs

+  | LfDoesNotSupportPkgImports
+  | Testing String
+  | Trace String --to insert callsite, for when reason unclear
+  | Combined [NoPkgImportsReason]


Does it ever make sense to nest these reasons? If not maybe you could stratify the types have have another type for a list of reasons. Or are you doing it this way for efficiency reasons? But then you could still use the free monoid.

paulbrauner-da · 2025-09-18T17:28:11Z

sdk/compiler/daml-lf-ast/src/DA/Daml/LF/Ast/Util.hs

+cannot get the genereated list by DA.Daml.StablePackages because that would
+introduce a circular dependency (DA.Daml.StablePackages depends on the encoder).
+If changes occur to the stablepackages, we can bootstrap this list: compile with
+current list of stable packages, observe hash, add has to list below. Since new


Suggested change

current list of stable packages, observe hash, add has to list below. Since new

current list of stable packages, observe hash, add hash to list below. Since new

paulbrauner-da · 2025-09-18T17:28:44Z

sdk/compiler/daml-lf-ast/src/DA/Daml/LF/Ast/Util.hs

        upgradedPackageId = Nothing
+
+{-
+We put a list of stablepackages here, to be used by the proto encoder. These


Is it feasible to have a test that this list is consistent with the hashes of stable packages?

paulbrauner-da · 2025-09-18T17:31:05Z

sdk/compiler/daml-lf-ast/src/DA/Daml/LF/Ast/Version.hs

+ifSupports :: MonadReader r m => Feature -> Getting Version r Version -> m a -> m a -> m a
+ifSupports f = ifVersion (`supports` f)
+
+assertSupports :: MonadReader r m => Feature -> Getting Version r Version -> (Version -> m ()) -> m ()


Maybe you could call this whenNotSupports for consistency with the when of monads. Same below.

paulbrauner-da · 2025-09-18T17:33:44Z

sdk/compiler/daml-lf-ast/src/DA/Daml/LF/Ast/Version.hs

+
+
+-- The extended implementation
+ifVersionWith :: MonadReader r m


Nit: here and below I think it would be a bit more natural to take the lens first, because this seems like the argument you're mostly likely to specialize on.

roger-bosman-da added 30 commits September 1, 2025 12:11

Initial implementation

f16f4ac

Initial logic, with "debugging packages" ifDebug Move to set, fix

Cabal changes

73e0bf1

@dylant-da comments

c4a7204

Pkg imports: Incremental builds

719152b

Comments @dylant-da

b954def

Add to damlRule

ea31949

Move to maybe

3b7b17f

Disable tests incremental builds

439c0d2

Reenable tests

14bc9d6

Minimal (Module, Imports) with tests passing

df15366

Test still pass 🙏

87d3ec4

Tests pass 🙏

a55ba2a

Tests still pass 🙏

9815d7d

Now test fail 😔

cf8a5b6

Switch to ImportMap (no proto changes yet)

e57028a

Sort-of decode implementation

cd01443

Move encode to Reader of State

27afa7a

Incorporate ifSupports(M)

6c2e753

Conditional pkg import interning

fec8767

Haskell side

2d94f3f

Q

Decoding, scala side

1e638df

Stable packages list

f5c3f40

Scala encoder: add pkgimports

d1c8d22

Passing build not test

37787f5

Maybe -> Either (build passing)

5cea6f4

Buidling

a6f6880

fmt.sh

aa8cc74

Initial implementation

e34a398

Initial logic, with "debugging packages" ifDebug Move to set, fix

Cabal changes

4d1dcf2

@dylant-da comments

bb38d9a

roger-bosman-da added 2 commits September 8, 2025 10:27

Fix merge

869166e

Mandate pkg interning on haskell side

3a5d82b

roger-bosman-da added 9 commits September 8, 2025 14:05

Merge branch 'main' into roger/package-reftable

d56b825

Disable outdated test for 2.dev

be8878d

platform-independence.dar-hash

de50a28

Enforce on scala side (this breaks scala encoder)

9012ff7

pkg import table scala logic

0528095

Temporarely normalize relative path, to fix

105c321

Merge branch 'main' into roger/package-reftable

201376f

Fix PackageImports

652f78e

Remove logging as it trips test

378b5a9

roger-bosman-da force-pushed the roger/package-reftable branch 2 times, most recently from 4ca9c6f to 6583491 Compare September 12, 2025 08:47

Add imports to EncodeModule rule, fixing tests?🙏

ea1fec0

lg Add imports to EncodeModule rule, fixing tests?🙏

roger-bosman-da force-pushed the roger/package-reftable branch from 6583491 to ea1fec0 Compare September 12, 2025 08:48

Cleanup unused/comments

2b36b76

roger-bosman-da commented Sep 12, 2025

View reviewed changes

roger-bosman-da force-pushed the roger/package-reftable branch from 1a843c2 to 2b36b76 Compare September 12, 2025 15:22

roger-bosman-da added 2 commits September 17, 2025 13:30

Error on Left in encoding, add todo to generateimports

2cb7cec

Fix mergeImportedPackages'

43caab0

roger-bosman-da marked this pull request as ready for review September 17, 2025 13:22

roger-bosman-da requested a review from a team as a code owner September 17, 2025 13:22

roger-bosman-da added 2 commits September 17, 2025 17:31

Make generatePackageImports conditional on lf

31e0d3c

PID.dar-hash

0bd1bcf

remyhaemmerle-da reviewed Sep 18, 2025

View reviewed changes

paulbrauner-da reviewed Sep 18, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

SG19-5: enforce package import table #21847

SG19-5: enforce package import table #21847

Uh oh!

roger-bosman-da commented Sep 3, 2025

Uh oh!

roger-bosman-da commented Sep 8, 2025

Uh oh!

azure-pipelines bot commented Sep 8, 2025

Uh oh!

roger-bosman-da Sep 12, 2025

Uh oh!

roger-bosman-da Sep 12, 2025

Uh oh!

roger-bosman-da Sep 12, 2025

Uh oh!

remyhaemmerle-da left a comment

Uh oh!

remyhaemmerle-da Sep 18, 2025

Uh oh!

remyhaemmerle-da Sep 18, 2025

Uh oh!

paulbrauner-da left a comment

Uh oh!

paulbrauner-da Sep 18, 2025

Uh oh!

paulbrauner-da Sep 18, 2025

Uh oh!

paulbrauner-da Sep 18, 2025

Uh oh!

paulbrauner-da Sep 18, 2025

Uh oh!

paulbrauner-da Sep 18, 2025

Uh oh!

Uh oh!

		string no_imported_packages_reason = 8;
		PackageImports package_imports = 9;

	current list of stable packages, observe hash, add has to list below. Since new
	current list of stable packages, observe hash, add hash to list below. Since new



		-- The extended implementation
		ifVersionWith :: MonadReader r m

SG19-5: enforce package import table #21847

Are you sure you want to change the base?

SG19-5: enforce package import table #21847

Uh oh!

Conversation

roger-bosman-da commented Sep 3, 2025

Uh oh!

roger-bosman-da commented Sep 8, 2025

Uh oh!

azure-pipelines bot commented Sep 8, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

remyhaemmerle-da left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paulbrauner-da left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!