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

feat: make sysroot a feature #315

Merged
merged 1 commit into from
Feb 12, 2025
Merged

feat: make sysroot a feature #315

merged 1 commit into from
Feb 12, 2025

Conversation

cloudhan
Copy link
Collaborator

@cloudhan cloudhan commented Jan 28, 2025

Possibly supersede #313.

This should be fully address and tested in the future cross compile support.

@cloudhan
Copy link
Collaborator Author

@lukasoyen I think this will work.

@lukasoyen
Copy link

I do like this and would rebase my PR once this is merged. Unfortunately this alone does not fix my issue.

I am having a hard time coming up with a reproducer. But my problem comes from using a rules_cc based toolchain like this one. These new toolchains do not set the builtin_sysroot attribute.

@cloudhan
Copy link
Collaborator Author

cloudhan commented Feb 2, 2025

I added some flag tests for sysroot. But since it requires us to register a dummy toolchain with sysroot in it, so without a large refactor, I don't we are ready for the merge...

Or maybe use dev_dependency=True and make the toolchain resolution more strict. Not sure if it worth the effort.

@cloudhan cloudhan merged commit c035fa1 into main Feb 12, 2025
17 checks passed
@cloudhan cloudhan deleted the cloudhan/sysroot branch February 12, 2025 15:16
@cloudhan
Copy link
Collaborator Author

Due to complexity for test, leave it for future cross compiling.

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