Only run lalrpop generation on ubuntu-latest #4132
Conversation
|
I wish if it were true. Mysteriously, they generate different file hash. Check hash values from any other builds: https://github.com/RustPython/RustPython/runs/7983393352?check_suite_focus=true |
|
crlf vs lf issue (hopefully) :) |
|
Well that's strange.... windows: ubuntu |
030dc18 to
26a7e3f
Compare
|
ohohoho, it was that AND actions/cache#888. updated to actions/cache@v3 to fix it. |
|
I give up, I think it's easier to cache the lalrpop binary for each platform. |
037c541 to
4621331
Compare
|
Okay, looks like it doesn't actually cache it yet but I'm okay with this for now - I've requested lalrpop be added to cargo-quickinstall's binary package cache so in the future we can use that. |
|
I know I've not been around for a couple months, but I really do still think that just checking the generated file into the repository is the easiest way to do this - it cuts down on compile times for everyone, and it just works for first-time contributors (and ci, hahahaaa): no having to remember to pass the correct feature, or install an external tool. Just |
6322e68 to
aae43a9
Compare
|
(ok now it does really cache the binary in the gh actions cache) |
11e2f52 to
ad6a1c8
Compare
|
previously, lalrpop job was only checking hash if the hash didn't change and used cached crate install for lalrpop even when lalrpop is required(baptiste0928/cargo-install@v1 does it - the reason CI was still using command instead of feature). Check the previous CI results. running cost for lalrpop was very low unless lalrpop was actually required. If you care about CI cost, this changed version increases the CI cost. The CI cost was almost same as before without this change.
#4129 also do it. please check it. |
|
Oh, I found you made your own cache of lalrpop binary. |
|
optional lalrpop feature of parser revealed not working well. going back to this way would be better |
ad6a1c8 to
1dfefa6
Compare
|
Is this issue relevant now that Parser lives in a separate repo? (Should it be moved there?) |
|
The generated parser is bundled again in the new repository |
Unless I'm misunderstanding something in the gh actions config, running lalrpop on windows and linux doesn't actually do anything - the cache key doesn't have the os in it, so it'll just overwrite each other and it's doing more work than it needs to.