RustPython
RustPython copied to clipboard
Only run lalrpop generation on ubuntu-latest
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.
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 You will find this version of CI fails if you change python.lalrpop.
crlf vs lf issue (hopefully) :)
Well that's strange....
windows:

ubuntu

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.
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 cargo build and you have an interpreter.
(ok now it does really cache the binary in the gh actions cache)
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.
no having to remember to pass the correct feature, or install an external tool
#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
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