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

Improve toolchain integration #492

Merged
merged 50 commits into from
Aug 30, 2023
Merged

Improve toolchain integration #492

merged 50 commits into from
Aug 30, 2023

Conversation

ehaas
Copy link
Collaborator

@ehaas ehaas commented Jul 14, 2023

Work in progress - Add some additional arguments to the linker + path discovery for file components

Next step will be figuring out where GCC is installed

@ehaas ehaas force-pushed the toolchain branch 3 times, most recently from 0ee9656 to 14602f5 Compare August 17, 2023 06:43
@ehaas ehaas marked this pull request as ready for review August 18, 2023 06:07
@ehaas
Copy link
Collaborator Author

ehaas commented Aug 18, 2023

@Vexu Ok I think this is finally ready for review. There's a lot here, would it be easier if I tried to break it up into a bunch of smaller PRs that build on each other? The logic is mostly adapted from clang; with the caveat that there are probably lots of bugs in the translation because I can't really test it on a wide variety of systems.

Copy link
Owner

@Vexu Vexu left a comment

Choose a reason for hiding this comment

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

Ok I think this is finally ready for review. There's a lot here, would it be easier if I tried to break it up into a bunch of smaller PRs that build on each other? The logic is mostly adapted from clang; with the caveat that there are probably lots of bugs in the translation because I can't really test it on a wide variety of systems.

Skimming through I didn't spot anything obviously bad, I'm fine with merging it as is and fixing things as they're found unless you want more throughout reviews?

src/main.zig Outdated Show resolved Hide resolved
src/main.zig Outdated Show resolved Hide resolved
@ehaas
Copy link
Collaborator Author

ehaas commented Aug 26, 2023

Ok, made the suggested changes. I'm ok with merging it as-is (assuming it works on your machine) and then fixing issues as they come up when more people start using it on a broader variety of systems.

@Vexu Vexu merged commit a6a52fd into Vexu:master Aug 30, 2023
3 checks passed
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