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

cmd/build+compile: allow opt-out of dependents gathering #5038

Merged
merged 2 commits into from Aug 23, 2022

Conversation

srenatus
Copy link
Contributor

@srenatus srenatus commented Aug 23, 2022

With .WithPruneUnused(true), the compiler (of the compile package) no longer
collects dependents of its entrypoints.

The resulting bundle, if used with the wasm target, will no longer be
semantically equivalent to the bundle built with the rego target.

Since we're unable to have entrypoints for functions, this allows building
modules that we couldn't build before.

Fixes #5035.

@srenatus srenatus changed the title compile: allow opt-out of dependents gathering cmd/build+compile: allow opt-out of dependents gathering Aug 23, 2022
@srenatus srenatus force-pushed the sr/compile/issue-5035 branch 2 times, most recently from e01d770 to 358eb5e Compare August 23, 2022 07:29
@srenatus srenatus marked this pull request as ready for review August 23, 2022 07:43
anderseknert
anderseknert previously approved these changes Aug 23, 2022
Copy link
Member

@anderseknert anderseknert left a comment

Choose a reason for hiding this comment

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

LGTM!

//
// Notably this includes functions (they can't be entrypoints) and causes
// the built bundle to no longer be semantically equivalent to the bundle built
// without wasm, or optimizations.}
Copy link
Member

Choose a reason for hiding this comment

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

Might be worth mentioning this option is for -t plan just as much?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's no real equivalence there, I thought... but yeah as it stands, the "optimizations" part is also misleading. I'll update the comment. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated. WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

Looks great!

With `.WithPruneUnused(true)`, the compiler (of the compile package) no longer
collects dependents of its entrypoints.

The resulting bundle, if used with the wasm target, will no longer be
semantically equivalent to the bundle built with the rego target.

Since we're unable to have entrypoints for functions, this allows building
modules that we couldn't build before. See open-policy-agent#5035.

Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
@srenatus srenatus merged commit 3fbe069 into open-policy-agent:main Aug 23, 2022
@srenatus srenatus deleted the sr/compile/issue-5035 branch August 23, 2022 08:31
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.

Add an option to relax semantic equivalence requirement on compile package
2 participants