Skip to content

build: use foreign_cc for luajit#6168

Merged
htuch merged 9 commits intoenvoyproxy:masterfrom
lizan:luajit_external
Mar 6, 2019
Merged

build: use foreign_cc for luajit#6168
htuch merged 9 commits intoenvoyproxy:masterfrom
lizan:luajit_external

Conversation

@lizan
Copy link
Member

@lizan lizan commented Mar 5, 2019

Signed-off-by: Lizan Zhou lizan@tetrate.io

Description:
Remove the last prebuilt dependencies and switches to foreign_cc with a slight wrapper script.

Risk Level: Low
Testing: CI
Docs Changes:
Release Notes:

Signed-off-by: Lizan Zhou <lizan@tetrate.io>
@lizan lizan requested a review from htuch March 5, 2019 02:33
@lizan
Copy link
Member Author

lizan commented Mar 5, 2019

@htuch while marking this as draft this is ready for removing LuaJIT prebuilt part, let me know whether you want to include removing all envoy_deps stuff in the same PR or a different one.

lizan added 2 commits March 5, 2019 03:00
Signed-off-by: Lizan Zhou <lizan@tetrate.io>
Signed-off-by: Lizan Zhou <lizan@tetrate.io>
Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

LGTM, this is great. Nuke all the old prebuilt, Envoy deps and other machinery for build recipes, I'm so happy to see it go!

@mattklein123
Copy link
Member

This is so exciting. I agree, please kill all the old infra. Lots to delete.

lizan added 4 commits March 5, 2019 20:45
Signed-off-by: Lizan Zhou <lizan@tetrate.io>
Signed-off-by: Lizan Zhou <lizan@tetrate.io>
Signed-off-by: Lizan Zhou <lizan@tetrate.io>
htuch
htuch previously approved these changes Mar 5, 2019
Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

Can you fix CI and we can ship this? I'd like to have this in place while attempting to fix #6061, which in turn is blocking #5953.

Signed-off-by: Lizan Zhou <lizan@tetrate.io>
@lizan lizan marked this pull request as ready for review March 6, 2019 01:51
Signed-off-by: Lizan Zhou <lizan@tetrate.io>
@lizan
Copy link
Member Author

lizan commented Mar 6, 2019

@htuch this should be good to go.

Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

The end of an era! Thanks @lizan and also to @irengrig for delivering rules_foreign_cc, totally awesome.

@htuch htuch merged commit 3d3f3af into envoyproxy:master Mar 6, 2019
fredlas pushed a commit to fredlas/envoy that referenced this pull request Mar 6, 2019
Remove the last prebuilt dependencies and switches to foreign_cc with a slight wrapper script.

Risk Level: Low
Testing: CI

Signed-off-by: Lizan Zhou <lizan@tetrate.io>
htuch pushed a commit that referenced this pull request Mar 6, 2019
Follow up of #6168.

Risk Level: Low

Signed-off-by: Lizan Zhou <lizan@tetrate.io>
PiotrSikora added a commit to PiotrSikora/envoy that referenced this pull request May 17, 2019
Migration from the build recipes to foreign_cc rules resulted
in dependencies being built with different compiler flags.

Among other things, those compiler flags were added:

    -ffunction-sections -fdata-sections

use of which leads to LuaJIT panicking:

    PANIC: unprotected error in call to Lua API

and Envoy subsequently crashing.

Broken in envoyproxy#6168.

Fixes istio/istio#13722.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
lizan pushed a commit that referenced this pull request May 31, 2019
Migration from the build recipes to foreign_cc rules resulted
in dependencies being built with different compiler flags.

Among other things, those compiler flags were added:

    -ffunction-sections -fdata-sections

use of which leads to LuaJIT panicking:

    PANIC: unprotected error in call to Lua API

and Envoy subsequently crashing.

Broken in #6168.

Fixes istio/istio#13722.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
PiotrSikora added a commit to istio/envoy that referenced this pull request Jun 3, 2019
Migration from the build recipes to foreign_cc rules resulted
in dependencies being built with different compiler flags.

Among other things, those compiler flags were added:

    -ffunction-sections -fdata-sections

use of which leads to LuaJIT panicking:

    PANIC: unprotected error in call to Lua API

and Envoy subsequently crashing.

Broken in envoyproxy#6168.

Fixes istio/istio#13722.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
+ os.environ["MACOSX_DEPLOYMENT_TARGET"] = "10.6"
+ os.environ["DEFAULT_CC"] = os.environ.get("CC", "")
+ os.environ["TARGET_CFLAGS"] = os.environ.get("CFLAGS", "")
+ os.environ["TARGET_LDFLAGS"] = os.environ.get("CFLAGS", "")
Copy link

Choose a reason for hiding this comment

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

@lizan Is this intentional or just a bug carried over?
Isn't TARGET_LDFLAGS supposed to be overridden by os.environ.get("LDFLAGS")

was TARGET_LDFLAGS=${CFLAGS} just to address that MacOSX bug mentioned in old code comment?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, not sure, this is just to replicate what we had in luajit.sh.

What is the macOS bug, the MACOSX_DEPLOYMENT_TARGET one is not relevant to this environment variable.

Copy link

Choose a reason for hiding this comment

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

The

-   MACOSX_DEPLOYMENT_TARGET=10.6 DEFAULT_CC=${CC} TARGET_CFLAGS=${CFLAGS} TARGET_LDFLAGS=${CFLAGS} 

Was the first place where TARGET_LDFLAGS was overridden with CFLAGS, and I think this small mistake was carried over in

+    os.environ["TARGET_LDFLAGS"] = os.environ.get("CFLAGS", "")

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.

4 participants