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

Go: run go build after code generation #2463

Closed
1 task done
eladb opened this issue Jan 21, 2021 · 1 comment · Fixed by #2485
Closed
1 task done

Go: run go build after code generation #2463

eladb opened this issue Jan 21, 2021 · 1 comment · Fixed by #2485
Assignees
Labels
feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged.

Comments

@eladb
Copy link
Contributor

eladb commented Jan 21, 2021

🚀 Feature Request

Affected Languages

  • golang

General Information

  • JSII Version: 1.17.1

To ensure that the generated go output is valid, I would recommend running go build on the resulting artifact so that errors will be discovered during build and not when the module is consumed.

Description

The issue #2457 uncovered a weakness in how go code is generated. The result was invalid but jsii-pacmak successfully finished.

Proposed Solution

By executing go build after code generation, we can ensure that the output compiles. Otherwise, this would have only be discovered when the module is consumed, which is definitely not a great experience.

@eladb eladb added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Jan 21, 2021
eladb pushed a commit that referenced this issue Jan 26, 2021
To ensure the generated go code is formatted and compiles by executing `go fmt`
and `go build` on the output directory.

In order to be able to do that within mono-repos (and in the jsii repo itself), we need to be able to resolve local dependencies (same as all other languages).

Resolves #2463
RomainMuller pushed a commit that referenced this issue Jan 28, 2021
To ensure the generated go code is formatted and compiles by executing `go fmt`
and `go build` on the output directory.

Similar to other languages, in order to be able to build the go module within mono-repos (and in the jsii repo itself), we need to be able to resolve local dependencies. This is done in the following way:

1. Create a list of `dist/go` candidates (based on the `jsii.outdir` relative to dependency package dirs)
2. Add to this list the current `outDir`, in case `--outdir` is used
3. Iterate over the module's dependencies (_recursively_) and for each candidate `dist` directory, check if we can find a `<goModule>/go.mod` file that contains a `module <moduleName>` directive.
4. If we find one, we append a `replace` directive with a relative path to our own `go.mod` which tells `go fmt` and `go build` to find the dependency locally.
5. At the end, we restore the original `go.mod` file so that the published one will not include the `replace` directives.

TESTING: this is primarily tested using the `test:build` target of `jsii-pacmak` which generates & builds the go code for the "calc" fixture (in all pacmak execution variants).

NOTE: running `go fmt` on the generated sources turned out to be problematic because sources are generated in parallel by default, and therefore it was not possible to resolve local dependencies at that time (which is required since `go fmt` processes `go.mod`).

Resolves #2463
@github-actions
Copy link
Contributor

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants