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

DO NOT MERGE - showing diff between TestModels/SharedMakefile.mk and the new SmithyDafnyMakefile.mk #326

Closed
wants to merge 3 commits into from

Conversation

robin-aws
Copy link
Contributor

@robin-aws robin-aws commented Mar 4, 2024

See #325. This PR copies the contents of SmithyDafnyMakefile.mk over top of TestModels/SharedMakefile.mk, to show what changes as the former is adopted.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@@ -162,15 +224,14 @@ transpile_test:
-functionSyntax:3 \
-useRuntimeLib \
-out $(OUT) \
-library:$(PROJECT_ROOT)/dafny-dependencies/StandardLibrary/src/Index.dfy \
$(if $(strip $(STD_LIBRARY)) , -library:$(PROJECT_ROOT)/$(STD_LIBRARY)/src/Index.dfy, ) \
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, what is this if strip logic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem this is solving is that the StandardLibrary is automatically considered a dependency of all libraries, but it can't be when building the StandardLibrary itself.

The TestModels folder's solution is to override lots of targets in StandardLibrary/Makefile to do nothing. The MPL, ESDK and DB ESDK use this solution instead: only consider the StandardLibrary a dependency if STD_LIBRARY is empty (after stripping whitespace, which I gather is easy to introduce in make variables if you're not careful).

I opted to adopt the latter solution since it aligned a little better with reality and would cause less churn when adopting the SmithyDafnyMakefile.mk in the MPL, ESDK and DB ESDK.

Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest a comment somewhere.

TestModels/SharedMakefile.mk Show resolved Hide resolved
TestModels/SharedMakefile.mk Show resolved Hide resolved
TestModels/SharedMakefile.mk Show resolved Hide resolved
robin-aws added a commit that referenced this pull request Mar 5, 2024
Factors the content of `TestModels/SharedMakefile.mk` to a new top-level `SmithyDafnyMakefile.mk`, which is actually intended to be reused by any `smithy-dafny` project using the CLI through a submodule. Projects will still likely want their own repository-specific `SharedMakefile.mk` in order to set a few variables that are the same for all libraries, which is true for the `TestModels` subfolder here too.

Additional make targets and improvements from https://github.com/aws/aws-cryptographic-material-providers-library and general cleanup are also applied. Highlights:

* Delete `LIBRARIES`, which is unused and was replaced by `PROJECT_DEPENDENCIES`.
* Delete `SMITHY_NAMESPACE`, which is unused and was replaced by `SERVICE_NAMESPACE_<service>`
* Replace `STANDARD_LIBRARY_PATH` with `STD_LIBRARY`.
  * The test models were avoiding the same issue with the standard library trying to build itself by overriding the `transpile_***` targets to not build dependencies. Both mechanisms at once is redundant but safe.
* Introduce `MAX_RESOURCE_COUNT` with a default of `10000000`
* Tweak `GRADLEW` to be the variable to use for building Java runtimes, since the TestModels don't have a `gradlew` in each `runtimes/java` directory, but other projects like the MPL do. Running the smithy-dafny CLI always uses the copy in `codegen`.
* Don't also build `_polymorph_wrapped` by default, just have `TestModels/SharedMakefile.mk` add that instead.
* Invoke `_polymorph_dependencies` before building the current library, since that's necessary in some cases (and was already changed in MPL's makefile)

See #326 for a diff of SmithyDafnyMakefile.mk with the old contents of TestModels/SharedMakefile.mk.
@robin-aws robin-aws closed this Mar 5, 2024
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.

2 participants