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

sdk/go: Move Output implementation to internal #13495

Merged
merged 1 commit into from
Aug 1, 2023

Conversation

abhinav
Copy link
Contributor

@abhinav abhinav commented Jul 14, 2023

Note to reviewers:
GitHub's PR view will show a lot more for this change
than the actual code added/removed.
Consider opening a git diff of this with,

git diff -C --diff-algorithm=minimal

This will correctly detect the bulk of the diff as
"copy a file and delete a bunch of stuff from it."


In preparation for implementing generic variants of Input and Output
in a pulumix subpackage, move some of the requisite types to internal/
so that we don't have a cyclic dependency between pulumi and pulumix.

This change is largely mechanical with adjustmenets made for
compilation.
For places in pulumi/ where internal state of an output was accessed,
new top-level functions were added to internal/
that expose this information.

Changes were noting explicitly:

  • OutputState was changed to record dependencies as []internal.Resource
    instead of []Resource because moving Resource into internal
    will end up moving most of pulumi/ into internal.
    We cast to/from internal.Resource as needed.
  • internal.AnyOutputType is a reflect.Type filled by sdk/go/pulumi
    at init() time.
    This is an annoying hack, but we can't move AnyOutput into internal.
  • internal.FullyResolvedTypes is similarly filled by sdk/go/pulumi
    at init() time.

To add confidence to this change being safe,
I used apidiff to compare the API of the go/pulumi package
befor and after this change.
The command reported no changes.

On top of that, I also ran go doc -all . with the before/after
and ran a text diff on the result. The following changes were reported:

  • Input is now an alias to internal.Input
  • Output is now an alias to internal.Output
  • OutputState is now an alias to internal.OutputState
  • OutputState.ApplyT, OutputState.ApplyTWithContext were deleted
    (except they're present on the alias so they're still available)
  • Resource now embeds internal.Resource

Resolves #13585

@abhinav abhinav added the impact/no-changelog-required This issue doesn't require a CHANGELOG update label Jul 14, 2023
Copy link
Contributor Author

abhinav commented Jul 14, 2023

@pulumi-bot
Copy link
Contributor

pulumi-bot commented Jul 14, 2023

Changelog

[uncommitted] (2023-07-26)

Miscellaneous

  • [sdk/go] Move some types to an internal package, re-exporting them from sdk/go/pulumi. This should have no meaningful effect on users of these APIs.
    #13495

@abhinav
Copy link
Contributor Author

abhinav commented Jul 14, 2023

bors try

bors bot added a commit that referenced this pull request Jul 14, 2023
@bors
Copy link
Contributor

bors bot commented Jul 14, 2023

try

Build failed:

@abhinav
Copy link
Contributor Author

abhinav commented Jul 14, 2023

Green:
image

@abhinav abhinav force-pushed the abhinav/go-generics-internalize branch 2 times, most recently from 7f12431 to fa82122 Compare July 17, 2023 16:06
@abhinav abhinav force-pushed the abhinav/go-generics-internalize branch 2 times, most recently from e733a6f to 5f202fb Compare July 18, 2023 23:18
@abhinav abhinav force-pushed the abhinav/go-generics-internalize branch 3 times, most recently from 896c4a3 to 44677d6 Compare July 26, 2023 00:51
@abhinav abhinav marked this pull request as ready for review July 26, 2023 00:52
@abhinav abhinav requested a review from a team July 26, 2023 00:52
**Note to reviewers**:
GitHub's PR view will show a lot more for this change
than the actual code added/removed.
Consider opening a `git diff` of this with,

    git diff -C --diff-algorithm=minimal

This will correctly detect the bulk of the diff as
"copy a file and delete a bunch of stuff from it."

---

In preparation for implementing generic variants of Input and Output
in a pulumix subpackage, move some of the requisite types to internal/
so that we don't have a cyclic dependency between pulumi and pulumix.

This change is largely mechanical with adjustmenets made for
compilation.
For places in pulumi/ where internal state of an output was accessed,
new top-level functions were added to internal/
that expose this information.

Changes were noting explicitly:

- `OutputState` was changed to record dependencies as `[]internal.Resource`
  instead of `[]Resource` because moving `Resource` into internal
  will end up moving most of pulumi/ into internal.
  We cast to/from internal.Resource as needed.
- `internal.AnyOutputType` is a `reflect.Type` filled by sdk/go/pulumi
  at `init()` time.
  This is an annoying hack, but we can't move AnyOutput into internal.
- `internal.FullyResolvedTypes` is similarly filled by sdk/go/pulumi
  at `init()` time.

To add confidence to this change being safe,
I used [apidiff] to compare the API of the go/pulumi package
befor and after this change.
The command reported no changes.

  [apidiff]: https://pkg.go.dev/golang.org/x/exp@v0.0.0-20230713183714-613f0c0eb8a1/cmd/apidiff

On top of that, I also ran `go doc -all .` with the before/after
and ran a text diff on the result. The following changes were reported:

- `Input` is now an alias to `internal.Input`
- `Output` is now an alias to `internal.Output`
- `OutputState` is now an alias to `internal.OutputState`
- `OutputState.ApplyT`, `OutputState.ApplyTWithContext` were deleted
  (except they're present on the alias so they're still available)
- `Resource` now embeds `internal.Resource`

Resolves #13585
@abhinav abhinav force-pushed the abhinav/go-generics-internalize branch from 44677d6 to 1912563 Compare July 26, 2023 02:00
@abhinav
Copy link
Contributor Author

abhinav commented Aug 1, 2023

bors merge

@bors
Copy link
Contributor

bors bot commented Aug 1, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit b37a204 into master Aug 1, 2023
52 checks passed
@bors bors bot deleted the abhinav/go-generics-internalize branch August 1, 2023 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/no-changelog-required This issue doesn't require a CHANGELOG update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Go Generics] Pull core types of sdk/go into internal package [Phase 1]
3 participants