Skip to content

fix(native_image): fix C++ toolchain env setup#72

Closed
fmeum wants to merge 3 commits intosgammon:mainfrom
fmeum:env
Closed

fix(native_image): fix C++ toolchain env setup#72
fmeum wants to merge 3 commits intosgammon:mainfrom
fmeum:env

Conversation

@fmeum
Copy link
Copy Markdown
Collaborator

@fmeum fmeum commented Aug 31, 2023

Instead of relying on the default shell env, get the environment
variables declared by the C++ toolchain. On macOS, additionally use
apple_support to pass in DEVELOPER_DIR.

Since GraalVM sanitizes the environment, all variables are translated
into -E flags.

Fixes #67

fmeum added 3 commits August 31, 2023 08:54
Make use of `Args` lazy string formatting to save memory.
Also pass in compiler options as repeated flags, not joined on spaces,
to prevent issues when compiler options contain spaces.
`depset` structures should be kept as flat as possible. C++ toolchain
files are also already `depset`s and thus shouldn't be passed into
`tools`.
Instead of relying on the default shell env, get the environment
variables declared by the C++ toolchain. On macOS, additionally use
`apple_support` to pass in `DEVELOPER_DIR`.

Since GraalVM sanitizes the environment, all variables are translated
into `-E` flags.
@fmeum fmeum requested a review from sgammon as a code owner August 31, 2023 13:18
@fmeum
Copy link
Copy Markdown
Collaborator Author

fmeum commented Aug 31, 2023

Stacked on #70 and #71

@sonarqubecloud
Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@fmeum
Copy link
Copy Markdown
Collaborator Author

fmeum commented Aug 31, 2023

@keith This is what I came up with based on your diff. It works in Bazel CI :⁠-⁠)

@sgammon sgammon added bug Something isn't working enhancement New feature or request labels Sep 1, 2023
@sgammon sgammon added this to the 1.0.0 milestone Sep 1, 2023
@sgammon sgammon added 🚧 WIP Work-in-progress, do not merge platform:darwin Issues relating to macOS platform:windows Issues relating to Windows and MSVC labels Sep 1, 2023
sgammon added a commit that referenced this pull request Sep 1, 2023
- fix: use legacy rules from legacy gvm
- chore: drop `MODULE-resolved.bzl`
- chore: update bzlmod lock
- chore: update lib/docs deps and rebuild docs
- chore: remove redundant calls in sample projects

Applied on top of #72

Signed-off-by: Sam Gammon <sam@elide.ventures>
sgammon added a commit that referenced this pull request Sep 1, 2023
- fix: use legacy rules from legacy gvm
- chore: drop `MODULE-resolved.bzl`
- chore: update bzlmod lock
- chore: update lib/docs deps and rebuild docs
- chore: remove redundant calls in sample projects

Applied on top of #72

Signed-off-by: Sam Gammon <sam@elide.ventures>
@sgammon sgammon mentioned this pull request Sep 1, 2023
@fmeum
Copy link
Copy Markdown
Collaborator Author

fmeum commented Sep 1, 2023

I can see how lambda and nested functions in general are problematic on Bazel 4. Unfortunately loading them conditionally isn't possible without either splitting out the non-legacy rules or using a Bazel version check in a repository rule and generating code.

We could also mimic what apple_support.run does and then directly control the way environment variables are passed to ctx.actions.run. Maybe @keith also knows a better solution.

@sgammon
Copy link
Copy Markdown
Owner

sgammon commented Sep 1, 2023

@fmeum see #80, i've performed the rule split and refactor in anticipation of that same problem. bazel4 is fixed, mac tests are working, and that PR now includes this one as well as #73

i hope we can move discussion to #80 where we can prep a release at 0.10.x 😄

as such closing this (but only as long as that PR works for your needs @fmeum -- if you hit issues we can always reopen this one.)

@sgammon sgammon closed this Sep 1, 2023
@sgammon sgammon removed the 🚧 WIP Work-in-progress, do not merge label Sep 1, 2023
@fmeum fmeum deleted the env branch September 1, 2023 09:09
sgammon added a commit that referenced this pull request Sep 2, 2023
- fix: use legacy rules from legacy gvm
- chore: drop `MODULE-resolved.bzl`
- chore: update bzlmod lock
- chore: update lib/docs deps and rebuild docs
- chore: remove redundant calls in sample projects

Applied on top of #72

Signed-off-by: Sam Gammon <sam@elide.ventures>
sgammon added a commit that referenced this pull request Sep 4, 2023
- fix: use legacy rules from legacy gvm
- chore: drop `MODULE-resolved.bzl`
- chore: update bzlmod lock
- chore: update lib/docs deps and rebuild docs
- chore: remove redundant calls in sample projects

Applied on top of #72

Signed-off-by: Sam Gammon <sam@elide.ventures>
sgammon added a commit that referenced this pull request Sep 8, 2023
- fix: use legacy rules from legacy gvm
- chore: drop `MODULE-resolved.bzl`
- chore: update bzlmod lock
- chore: update lib/docs deps and rebuild docs
- chore: remove redundant calls in sample projects

Applied on top of #72

Signed-off-by: Sam Gammon <sam@elide.ventures>
sgammon added a commit that referenced this pull request Sep 8, 2023
- fix: use legacy rules from legacy gvm
- chore: drop `MODULE-resolved.bzl`
- chore: update bzlmod lock
- chore: update lib/docs deps and rebuild docs
- chore: remove redundant calls in sample projects

Applied on top of #72

Signed-off-by: Sam Gammon <sam@elide.ventures>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request platform:darwin Issues relating to macOS platform:windows Issues relating to Windows and MSVC

Projects

Development

Successfully merging this pull request may close these issues.

Populate environment for macOS builds

2 participants