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

Revert "lua: allow using moonjit (#10265)" #14603

Closed
wants to merge 2 commits into from

Conversation

wrowe
Copy link
Contributor

@wrowe wrowe commented Jan 8, 2021

Commit Message: Revert "lua: allow using moonjit (#10265)"
Additional Description:

This reverts commit 2c03496.

See thread https://twitter.com/siddhesh_p/status/1308594269502885889?s=20 @siddhesh_p Sep 22
"It ought to generally be easy to revert to the upstream tree since
they're both [moonjit and luajit] (AFAIK) on the 2.1.x branch. I'll be
happy to help with the transition if needed."

This is no longer maintained code, there is no way for envoy project to
assume responsibility, and after many months, no maintainers have
stepped forward. PR's to luajit effort are one appropriate way forward.

Resolves: #13539

With apologies to @iii-i, this appears to be an abandoned solution.

Signed-off-by: William A Rowe Jr wrowe@vmware.com

Risk Level: low-ish (s390x will become unsupported until changes are upstreamed to luajit)
Testing: local+ci
Docs Changes: n/a
Release Notes:

This reverts commit 2c03496.

See thread https://twitter.com/siddhesh_p/status/1308594269502885889?s=20 @siddhesh_p Sep 22
"It ought to generally be easy to revert to the upstream tree since
they're both [moonjit and luajit] (AFAIK) on the 2.1.x branch. I'll be
happy to help with the transition if needed."

This is no longer maintained code, there is no way for envoy project to
assume responsibility, and after many months, no maintainers have
stepped forward. PR's to luajit effort are one appropriate way forward.

Resolves: envoyproxy#13539

Signed-off-by: William A Rowe Jr <wrowe@vmware.com>
@repokitteh-read-only repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Jan 8, 2021
@repokitteh-read-only
Copy link

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).

🐱

Caused by: #14603 was opened by wrowe.

see: more, trace.

Signed-off-by: William A Rowe Jr <wrowe@vmware.com>
@mattklein123 mattklein123 self-assigned this Jan 8, 2021
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Thanks. Can you add a specific release note about dropping support for this? Also, check CI?

/wait

@wrowe wrowe marked this pull request as draft January 13, 2021 22:34
Base automatically changed from master to main January 15, 2021 23:02
@repokitteh-read-only
Copy link

🙀 Error while processing event:

evaluation error
error: context deadline exceeded
🐱

Caused by: #14603 was edited by mattklein123.

see: more, trace.

@wrowe
Copy link
Contributor Author

wrowe commented Jan 17, 2021

I've dropped this, as noted in the issue, to draft status.

We may want to retain the logic for electing a lua engine. The two most frequently updated and actively maintained are the Lua project, and the LuaJIT fork. The forks of LuaJIT I'm researching, but most seem largely inactive. Unfortunately this is a federation of single-maintainer efforts, which isn't conducive to a stable dependency. Until we decide a way forward, I want to park this PR for 3-4 weeks and research where these communities converge and diverge.

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Feb 22, 2021
@github-actions
Copy link

github-actions bot commented Mar 2, 2021

This pull request has been automatically closed because it has not had activity in the last 37 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@github-actions github-actions bot closed this Mar 2, 2021
@moderation
Copy link
Contributor

@wrowe can we revive this PR? As per chats with @htuch and @mattklein123 there is consensus with the @envoyproxy/dependency-shepherds that we should remove Moonjit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deps Approval required for changes to Envoy's external dependencies stale stalebot believes this issue/PR has not been touched recently waiting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deprecate moonjit forked component of envoy in favor of luajit
3 participants