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 CI github actions workflow to verify builds on pull_request #86

Merged
merged 1 commit into from
Jul 27, 2023

Conversation

tairov
Copy link
Contributor

@tairov tairov commented Jul 25, 2023

This CI workflow provides a system that ensures every change made to the master branch, specifically to Makefiles, *.h, or C source files, doesn't break the build process, improving the reliability and stability of the project.

It'd be nice to have basic build validation in order to avoid issues on different platforms like this one : #80

Builds validation platforms:
Linux : ✅
MacOS : ✅
Windows : ✅

@tairov
Copy link
Contributor Author

tairov commented Jul 25, 2023

CI pipeline looks good : https://github.com/tairov/llama2.c/actions/runs/5662603534

@karpathy
Copy link
Owner

Actually I like this, but few things:

  • is windows somehow harder to include?
  • can we make naming inside Makefile consistent?
  • can we make the actions specific to this repo? e.g. there are no header files so there is no need to include anything .h

@karpathy
Copy link
Owner

(By consistent I mean no underscores in target names, e.g. just make it clangomp or something?)

@tairov
Copy link
Contributor Author

tairov commented Jul 25, 2023

I agree regarding naming consistency -- Fixed.
Windows builds turned out to be a bit harder to verify on my Mac M1, I'm looking for a solution to reproduce environment locally.
I thought, maybe someone experienced with Windows msbuild can give a hand, if not, I'll sort this out till tomorrow I hope

@tairov
Copy link
Contributor Author

tairov commented Jul 26, 2023

@karpathy added windows platform build job (pipeline : https://github.com/tairov/llama2.c/actions/runs/5663670418 )

Erorrs with Windows builds ignored at the moment:

continue-on-error: true

Seems that compilations on Windows are not working via CL.exe at least , #91

Also removed irrelevant *.h targets from trigger paths

@tairov
Copy link
Contributor Author

tairov commented Jul 26, 2023

Adapted build.yml to latest changes in master ( tuneable compiler config )

@tairov tairov force-pushed the master branch 2 times, most recently from 2b8ca1d to 329cfd3 Compare July 27, 2023 10:56
@tairov
Copy link
Contributor Author

tairov commented Jul 27, 2023

Fixed builds for windows . Now CI workflow uses build_msvc.bat

@tairov tairov force-pushed the master branch 2 times, most recently from a79d92a to 956be53 Compare July 27, 2023 12:23
@karpathy
Copy link
Owner

@tairov please let me know when you think this could be good to merge. CI would be very nice!

@tairov
Copy link
Contributor Author

tairov commented Jul 27, 2023

Code rebased to catch up with upstream/master

@karpathy it's ready to be merged.

On my fork I'm using this to check builds for multiple platforms instantly - linux, macos, window (x86, 64, arm) very convenient.
Also I've adapted windows steps to use .\build_msvc.bat already.

Here is example how action execution will look like once a PR created in the repo that propose changes in any following files *.c, *.h, Makefile

@karpathy karpathy merged commit cc66a20 into karpathy:master Jul 27, 2023
@karpathy
Copy link
Owner

Exciting, thank you!

vinhtran2611 pushed a commit to vinhtran2611/llama2.c that referenced this pull request Jan 20, 2024
Add CI github actions workflow to verify builds on pull_request
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.

None yet

2 participants