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

feat(go): require go 1.16, use native embed #2603

Merged
merged 3 commits into from
Feb 25, 2021
Merged

Conversation

RomainMuller
Copy link
Contributor

Instead of generating code to build byte slices with the embedded
assets, upgraded the baseline go requirement to go 1.16, and use the new
native embed feature released as part of this go release.

This includes the update to jsii/superhain to the correct go release,
updates to the .github/workflows/main.yml workflow to ensure the
correct go version is installed in PR validation builds, and the changes
in jsii-pacmak and @jsii/go-runtime to leverage the feature.

This makes the code easier to deal with by IDEs (the byte slices were
otherwise very large and caused IDEs to occasionally choke on them),
and removes the risk we make a mistake in generating the slices. It
also makes the generated code easier to troubleshoot, as the tarball
is present as-is, and can be inspected without additional hoop jumping.

See: https://pkg.go.dev/embed


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@RomainMuller RomainMuller added language/go Regarding GoLang bindings effort/small Small work item – less than a day of effort module/pacmak Issues affecting the `jsii-pacmak` module module/runtime Issues affecting the `jsii-runtime` contribution/core This is a PR that came from AWS. labels Feb 24, 2021
@RomainMuller RomainMuller requested a review from a team February 24, 2021 11:00
@RomainMuller RomainMuller self-assigned this Feb 24, 2021
@RomainMuller
Copy link
Contributor Author

Note - this has a bootstrap paradox for the CodeBuild PR validation, as the jsii/superchain it uses would ship with a pre-1.16 go release. I'd manually solve this if/when we get consensus that this is something we want to move forward with.

Instead of generating code to build byte slices with the embedded
assets, upgraded the baseline go requirement to go 1.16, and use the new
native embed feature released as part of this go release.

This includes the update to `jsii/superhain` to the correct go release,
updates to the `.github/workflows/main.yml` workflow to ensure the
correct go version is installed in PR validation builds, and the changes
in `jsii-pacmak` and `@jsii/go-runtime` to leverage the feature.

See: https://pkg.go.dev/embed
@@ -344,7 +348,9 @@ export class RootPackage extends Package {
importGoModules(code, toImport);

code.line();
code.line('var once sync.Once');
code.line(`//go:embed ${tarballName(this.assembly)}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

wow, go:embed is weird

# Conflicts:
#	packages/jsii-pacmak/lib/targets/go.ts
#	packages/jsii-pacmak/test/generated-code/__snapshots__/target-go.test.ts.snap
@RomainMuller
Copy link
Contributor Author

I have manually resolved the bootstrap paradox at this stage (I think)

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject89A8053A-6Jw05QLuWWwe
  • Commit ID: 7d228b2
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

This was referenced Mar 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS. effort/small Small work item – less than a day of effort language/go Regarding GoLang bindings module/pacmak Issues affecting the `jsii-pacmak` module module/runtime Issues affecting the `jsii-runtime`
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants