-
Notifications
You must be signed in to change notification settings - Fork 249
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
Expand make variables in env #788
Conversation
df43a37
to
da14d17
Compare
bazel-contrib/rules_perl#27 seemed to be the blocker here. |
db41223
to
2b94179
Compare
@UebelAndre I think this is ready for review now :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Thanks for putting this together!
One suggestion if you don't mind 😅
Seems like CI ran into a flake? Commit amend and try again? |
Uh oh
|
This allows toolchains to bring along files and for those files to be referenced during the build. This is a slightly breaking change - any $s currently present in env var attributes which are not consumed by expand_location will need to be escaped with a second $, because Make-variable expansion requires that escaping. See the example change done to examples/third_party/openssl/BUILD.openssl.bazel
89636e7
to
41b1936
Compare
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
@googlebot I consent. |
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
This demonstrates that toolchains can be propery used in rules_foreign_cc rules. rules_perl doesn't currently have a Windows toolchain, but when it does, we can stop bringing along our own.
41b1936
to
74bf86d
Compare
This allows toolchains to bring along files and for those files to be
referenced during the build.
This is a slightly breaking change - any $s currently present in env var
attributes which are not consumed by expand_location will need to be
escaped with a second $, because Make-variable expansion requires that
escaping. See the example change done to examples/third_party/openssl/BUILD.openssl.bazel