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

Add toolchain executables to the PATH #987

Merged
merged 1 commit into from
Dec 10, 2022

Conversation

jheaff1
Copy link
Collaborator

@jheaff1 jheaff1 commented Nov 29, 2022

Previously, some toolchain executables, e.g. make, pkg-config etc. were not added to the PATH correctly. As such, even if make and pkg-config were built from source, the system executables were used.

This commit modifies the tools_files attribute of the rules_foreign_cc framework to instead be "tools_data", which provides path to the executable and the target that provides it.

@jheaff1 jheaff1 force-pushed the add_tools_to_path branch 4 times, most recently from e0bdced to 934fba7 Compare November 29, 2022 16:04
@jheaff1 jheaff1 marked this pull request as ready for review November 29, 2022 16:36
Previously, some toolchain executables, e.g. make, pkg-config etc. were
not added to the PATH correctly. As such, even if make and pkg-config
were built from source, the system executables were used.

This commit modifies the tools_files attribute of the rules_foreign_cc
framework to instead be "tools_data", which provides path to the
executable and the target that provides it.
@jheaff1
Copy link
Collaborator Author

jheaff1 commented Dec 3, 2022

@jsharpe @UebelAndre Any chance of a review? 😄

@jsharpe
Copy link
Member

jsharpe commented Dec 6, 2022

@jsharpe @UebelAndre Any chance of a review? 😄

Its on my todo list to take a look at this!

@jsharpe
Copy link
Member

jsharpe commented Dec 10, 2022

Apologies for the delay in reviewing this!

@jsharpe jsharpe merged commit 0ed27c1 into bazel-contrib:main Dec 10, 2022
@jheaff1
Copy link
Collaborator Author

jheaff1 commented Dec 10, 2022

Apologies for the delay in reviewing this!

No worries, thanks for taking the time to review it!

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.

2 participants