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.

Sign up for GitHub

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

Jump to bottom

dedupeTypes not run from Pulumi CLI generation #1508

Closed
danielrbradley opened this issue Dec 12, 2024 · 1 comment · Fixed by pulumi/pulumi#18138
Closed

dedupeTypes not run from Pulumi CLI generation #1508

danielrbradley opened this issue Dec 12, 2024 · 1 comment · Fixed by pulumi/pulumi#18138
Assignees
@lunaris
Labels
area/codegen Code generation kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed
Milestone
0.115

Comments

@danielrbradley
Copy link
Member

When running Java code generation via pulumi package gen-sdk, it doesn't run the dedupeTypes method which fixes certain aspects of the schema around duplicates and casing.

Similar to #1500, this method needs to be on the code path for both the standalone binary but also the GeneratePackage entry point for the Pulumi CLI.

Referece:

pulumi-java/pkg/cmd/pulumi-java-gen/generate.go

Line 56 in 6c1c330

pkgSpec, err := dedupTypes(rawPkgSpec)

@danielrbradley danielrbradley added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Dec 12, 2024
@danielrbradley
Copy link
Member Author

An initially spike of this show it's not entirely trivial because the deDup method requires a PackageSpec rather than a Package, so will need to be reworked.

Spike: https://github.com/pulumi/pulumi-java/compare/1508-move-dedup

@danielrbradley danielrbradley mentioned this issue Dec 12, 2024
Open
@justinvp justinvp removed the needs-triage Needs attention from the triage team label Dec 16, 2024
@lunaris lunaris self-assigned this Dec 20, 2024
lunaris added a commit that referenced this issue Dec 20, 2024
@lunaris
Centralize SDK generation logic
fcad284 
Presently, the majority of Java code generation is driven by the
`pulumi-java-gen` binary, since Java usage began before we had time to implement
the `Generate*` family of language host gRPC methods. Recently, these gRPC
methods were implemented, to support (among other things) conformance testing.
Unfortunately, while both routes end in `pkg/codegen/java`'s `Generate*`
functions, each had accumulated its own special "setup logic" ahead of the call
into `pkg/codegen`. This commit attempts to sort this out, pushing all that
logic into `pkg/codegen` so that both routes behave identically. As a result of
this, we should be able to deprecate `pulumi-java-gen` more safely when the time
comes, remove direct build-time dependencies on `pulumi-java` from
`pulumi/pulumi` and fix some issues that have arisen as a result of the historic
differences, such as #1404 (which looks like it may have already been fixed, but
this should cement it) and #1508.

Closes #1404
Fixes #1508
lunaris added a commit that referenced this issue Dec 20, 2024
@lunaris
Centralize SDK generation logic
ef22265 
Presently, the majority of Java code generation is driven by the
`pulumi-java-gen` binary, since Java usage began before we had time to implement
the `Generate*` family of language host gRPC methods. Recently, these gRPC
methods were implemented, to support (among other things) conformance testing.
Unfortunately, while both routes end in `pkg/codegen/java`'s `Generate*`
functions, each had accumulated its own special "setup logic" ahead of the call
into `pkg/codegen`. This commit attempts to sort this out, pushing all that
logic into `pkg/codegen` so that both routes behave identically. As a result of
this, we should be able to deprecate `pulumi-java-gen` more safely when the time
comes, remove direct build-time dependencies on `pulumi-java` from
`pulumi/pulumi` and fix some issues that have arisen as a result of the historic
differences, such as #1404 (which looks like it may have already been fixed, but
this should cement it) and #1508.

Closes #1404
Fixes #1508
lunaris added a commit that referenced this issue Dec 20, 2024
@lunaris
Centralize SDK generation logic
60713ce 
Presently, the majority of Java code generation is driven by the
`pulumi-java-gen` binary, since Java usage began before we had time to implement
the `Generate*` family of language host gRPC methods. Recently, these gRPC
methods were implemented, to support (among other things) conformance testing.
Unfortunately, while both routes end in `pkg/codegen/java`'s `Generate*`
functions, each had accumulated its own special "setup logic" ahead of the call
into `pkg/codegen`. This commit attempts to sort this out, pushing all that
logic into `pkg/codegen` so that both routes behave identically. As a result of
this, we should be able to deprecate `pulumi-java-gen` more safely when the time
comes, remove direct build-time dependencies on `pulumi-java` from
`pulumi/pulumi` and fix some issues that have arisen as a result of the historic
differences, such as #1404 (which looks like it may have already been fixed, but
this should cement it), and #1508

Closes #1404
Fixes #1508
@lunaris lunaris mentioned this issue Dec 20, 2024
Merged
lunaris added a commit that referenced this issue Dec 20, 2024
@lunaris
Centralize SDK generation logic
d1aed25 
Presently, the majority of Java code generation is driven by the
`pulumi-java-gen` binary, since Java usage began before we had time to implement
the `Generate*` family of language host gRPC methods. Recently, these gRPC
methods were implemented, to support (among other things) conformance testing.
Unfortunately, while both routes end in `pkg/codegen/java`'s `Generate*`
functions, each had accumulated its own special "setup logic" ahead of the call
into `pkg/codegen`. This commit attempts to sort this out, pushing all that
logic into `pkg/codegen` so that both routes behave identically. As a result of
this, we should be able to deprecate `pulumi-java-gen` more safely when the time
comes, remove direct build-time dependencies on `pulumi-java` from
`pulumi/pulumi` and fix some issues that have arisen as a result of the historic
differences, such as #1404 (which looks like it may have already been fixed, but
this should cement it), and #1508

Closes #1404
Fixes #1508
lunaris added a commit that referenced this issue Dec 20, 2024
@lunaris
Centralize SDK generation logic
123d176 
Presently, the majority of Java code generation is driven by the
`pulumi-java-gen` binary, since Java usage began before we had time to implement
the `Generate*` family of language host gRPC methods. Recently, these gRPC
methods were implemented, to support (among other things) conformance testing.
Unfortunately, while both routes end in `pkg/codegen/java`'s `Generate*`
functions, each had accumulated its own special "setup logic" ahead of the call
into `pkg/codegen`. This commit attempts to sort this out, pushing all that
logic into `pkg/codegen` so that both routes behave identically. As a result of
this, we should be able to deprecate `pulumi-java-gen` more safely when the time
comes, remove direct build-time dependencies on `pulumi-java` from
`pulumi/pulumi` and fix some issues that have arisen as a result of the historic
differences, such as #1404 (which looks like it may have already been fixed, but
this should cement it), and #1508

Closes #1404
Fixes #1508
lunaris added a commit that referenced this issue Dec 20, 2024
@lunaris
Centralize SDK generation logic (#1515)
d2a85f0 
Presently, the majority of Java code generation is driven by the
`pulumi-java-gen` binary, since Java usage began before we had time to
implement the `Generate*` family of language host gRPC methods.
Recently, these gRPC methods were implemented, to support (among other
things) conformance testing. Unfortunately, while both routes end in
`pkg/codegen/java`'s `Generate*` functions, each had accumulated its own
special "setup logic" ahead of the call into `pkg/codegen`. This commit
attempts to sort this out, pushing all that logic into `pkg/codegen` so
that both routes behave identically. As a result of this, we should be
able to deprecate `pulumi-java-gen` more safely when the time comes,
remove direct build-time dependencies on `pulumi-java` from
`pulumi/pulumi` and fix some issues that have arisen as a result of the
historic differences, such as #1404 (which looks like it may have
already been fixed, but this should cement it), and the Java side of
#1508.

Alongside new unit tests and existing conformance tests, this changeset
has been manually tested using `pulumi-azure-native` and changes akin to
those in pulumi/pulumi-azure-native#3776 (using a locally modified
`pulumi package gen-sdk` that can be mainstreamed when these changes
have been merged and released).

Closes #1404
Part of #1508
@justinvp justinvp added the area/codegen Code generation label Jan 3, 2025
@justinvp justinvp added this to the 0.115 milestone Jan 3, 2025
lunaris added a commit to pulumi/pulumi that referenced this issue Jan 3, 2025
@lunaris
Only use gRPC to communicate with the Java host
866e109 
Language hosts are an important part of the Pulumi ecosystem,
encapsulating language-specific concerns such as program execution,
plugin hosting, and code generation, and exposing these as a gRPC
service.

Presently, we abuse the fact that the Java language host (along with all
other Pulumi-maintained language hosts) is written in Go to invoke some
of its functionality _directly_---that is, as linked in-memory function
calls---by having `pulumi/pulumi` depend on `pulumi/pulumi-java` in its
`go.mod`. This is undesirable for several reasons -- it creates
unnecessary coupling (indeed, a very annoying cyclic dependency),
requires special-casing in various parts of the CLI, and is hard to
test. In this change, therefore, we switch out these in-memory calls for
"proper" gRPC ones. Specifically:

* `pulumi convert` and `pulumi import` will now call `GenerateProject`
  and `GenerateProgram` (Programgen) over a gRPC wire.
* `pulumi package gen-sdk` will now call `GeneratePackage` (SDKgen) over
  a gRPC wire.
* `pulumi-java` is removed as a dependency from `go.mod`(s).
  `pulumi-java`'s version is now explicitly tracked in the
  `scripts/get-language-providers.sh` script (which also becomes
  simpler now we have no `go.mod`-dependency-hosts any more).

Aside from the fact that calls are now being made over a network
connection, behaviour should be identical:

* The gRPC implementations of `GenerateProgram` and `GenerateProject`
  (found in `pkg/cmd/pulumi-language-java/main.go` in `pulumi-java`)
  call the currently used in-memory function as-is, with no surrounding
  code except for deserialization of inputs and binding of PCL.
* The gRPC implementation of `GeneratePackage` performs a single
  operation aside from deserialization and binding -- name
  deduplication. This is the same function as that performed by the
  `pulumi-java-gen` binary that is currently used to generate Java SDKs
  (until now, `gen-sdk` has not been used _at all_---in-memory or
  otherwise---due to this and other issues; see
  pulumi/pulumi-java#1508). As a result, this should now be a suitable
  replacement for `pulumi-java-gen` and the correct implementation for
  `gen-sdk`.

With this, we can remove the cyclic dependency, get rid of
`pulumi-java-gen` and clean up several bits of bespoke code. Rock on,
Java!

> [!NOTE]
> Aside from Java conformance tests giving us confidence, `pulumi
> package gen-sdk` has been manually tested with `pulumi-azure-native`,
> which is referenced in pulumi/pulumi-java#1508.

Fixes pulumi-java#1508
lunaris added a commit to pulumi/pulumi that referenced this issue Jan 3, 2025
@lunaris
Only use gRPC to communicate with the Java host
2844902 
Language hosts are an important part of the Pulumi ecosystem,
encapsulating language-specific concerns such as program execution,
plugin hosting, and code generation, and exposing these as a gRPC
service.

Presently, we abuse the fact that the Java language host (along with all
other Pulumi-maintained language hosts) is written in Go to invoke some
of its functionality _directly_---that is, as linked in-memory function
calls---by having `pulumi/pulumi` depend on `pulumi/pulumi-java` in its
`go.mod`. This is undesirable for several reasons -- it creates
unnecessary coupling (indeed, a very annoying cyclic dependency),
requires special-casing in various parts of the CLI, and is hard to
test. In this change, therefore, we switch out these in-memory calls for
"proper" gRPC ones. Specifically:

* `pulumi convert` and `pulumi import` will now call `GenerateProject`
  and `GenerateProgram` (Programgen) over a gRPC wire.
* `pulumi package gen-sdk` will now call `GeneratePackage` (SDKgen) over
  a gRPC wire.
* `pulumi-java` is removed as a dependency from `go.mod`(s).
  `pulumi-java`'s version is now explicitly tracked in the
  `scripts/get-language-providers.sh` script (which also becomes
  simpler now we have no `go.mod`-dependency-hosts any more).

Aside from the fact that calls are now being made over a network
connection, behaviour should be identical:

* The gRPC implementations of `GenerateProgram` and `GenerateProject`
  (found in `pkg/cmd/pulumi-language-java/main.go` in `pulumi-java`)
  call the currently used in-memory function as-is, with no surrounding
  code except for deserialization of inputs and binding of PCL.
* The gRPC implementation of `GeneratePackage` performs a single
  operation aside from deserialization and binding -- name
  deduplication. This is the same function as that performed by the
  `pulumi-java-gen` binary that is currently used to generate Java SDKs
  (until now, `gen-sdk` has not been used _at all_---in-memory or
  otherwise---due to this and other issues; see
  pulumi/pulumi-java#1508). As a result, this should now be a suitable
  replacement for `pulumi-java-gen` and the correct implementation for
  `gen-sdk`.

With this, we can remove the cyclic dependency, get rid of
`pulumi-java-gen` and clean up several bits of bespoke code. Rock on,
Java!

> [!NOTE]
> Aside from Java conformance tests giving us confidence, `pulumi
> package gen-sdk` has been manually tested with `pulumi-azure-native`,
> which is referenced in pulumi/pulumi-java#1508.

Fixes pulumi/pulumi-java#1508
@lunaris lunaris mentioned this issue Jan 3, 2025
Merged
github-merge-queue bot pushed a commit to pulumi/pulumi that referenced this issue Jan 3, 2025
@lunaris
Only use gRPC to communicate with the Java host (#18138)
34eac65 
Language hosts are an important part of the Pulumi ecosystem,
encapsulating language-specific concerns such as program execution,
plugin hosting, and code generation, and exposing these as a gRPC
service.

Presently, we abuse the fact that the Java language host (along with all
other Pulumi-maintained language hosts) is written in Go to invoke some
of its functionality _directly_---that is, as linked in-memory function
calls---by having `pulumi/pulumi` depend on `pulumi/pulumi-java` in its
`go.mod`. This is undesirable for several reasons -- it creates
unnecessary coupling (indeed, a very annoying cyclic dependency),
requires special-casing in various parts of the CLI, and is hard to
test. In this change, therefore, we switch out these in-memory calls for
"proper" gRPC ones. Specifically:

* `pulumi convert` and `pulumi import` will now call `GenerateProject`
and `GenerateProgram` (Programgen) over a gRPC wire.
* `pulumi package gen-sdk` will now call `GeneratePackage` (SDKgen) over
a gRPC wire.
* `pulumi-java` is removed as a dependency from `go.mod`(s).
`pulumi-java`'s version is now explicitly tracked in the
`scripts/get-language-providers.sh` script (which also becomes simpler
now we have no `go.mod`-dependency-hosts any more).

Aside from the fact that calls are now being made over a network
connection, behaviour should be identical:

* The gRPC implementations of `GenerateProgram` and `GenerateProject`
(found in `pkg/cmd/pulumi-language-java/main.go` in `pulumi-java`) call
the currently used in-memory function as-is, with no surrounding code
except for deserialization of inputs and binding of PCL.
* The gRPC implementation of `GeneratePackage` performs a single
operation aside from deserialization and binding -- name deduplication.
This is the same function as that performed by the `pulumi-java-gen`
binary that is currently used to generate Java SDKs (until now,
`gen-sdk` has not been used _at all_---in-memory or otherwise---due to
this and other issues; see
pulumi/pulumi-java#1508). As a result, this
should now be a suitable replacement for `pulumi-java-gen` and the
correct implementation for `gen-sdk`.

With this, we can remove the cyclic dependency, get rid of
`pulumi-java-gen` and clean up several bits of bespoke code. Rock on,
Java!

> [!NOTE]
> Aside from Java conformance tests giving us confidence, `pulumi
package gen-sdk` has been manually tested with `pulumi-azure-native`,
which is referenced in
pulumi/pulumi-java#1508.

Fixes pulumi/pulumi-java#1508
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label Jan 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lunaris lunaris

Labels
area/codegen Code generation kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed
Projects
None yet
Milestone
0.115
Development

Successfully merging a pull request may close this issue.

Only use gRPC to communicate with the Java host pulumi/pulumi
4 participants
@lunaris @danielrbradley @justinvp @pulumi-bot