Conversation
|
@zuiderkwast we didn't want to add python as a dependency for building redis, that's why we're packaging commands.c as part of the project source code. Arguably, your suggestion doesn't break that, but i'm not sure i trust file system modification time. @yossigo WDYT? |
|
If not, then let's at least add the rule for commands.c so I can write |
|
that we can certainly do.. we actually had it, and i asked to remove it (together with the rule for building help.h). i'm still not confident we can't take your PR as is.. maybe i'm just paranoid, and in practice there's no way that the json files will have a newer time than command.c for naive users that just wanna build redis from a tar file or a git clone. p.s i do have some ideas about some GH action around commands.c generations or validations (e.g. it can generate it and compare to the copy in the branch). |
|
@oranagra I think the problem won't be the filesystem but inexperienced users who somehow get the files modified and don't have the Python dependency. |
|
If don't want to depend on Python and we're going to use JSON more, perhaps it's time to include a minimal C JSON library? There are some in ~400 LOC. For commands.c, we can even use one without Unicode support. Then we can rewrite the python script in C or we can let redis-server load the JSON files directly. |
|
@zuiderkwast I don't feel very comfortable doing this in C (and certainly not at runtime) only because of toolchain issues. Maybe we can adopt a middle way - include |
|
This PR is already a middle way, leaving commands.c checked in. I can add some make ifdefs based on |
Co-authored-by: Yossi Gottlieb <[email protected]>
With this rule, the script to generate commands.c from JSON runs whenever commands.o is built if any of commands/*.json are modified. Without such rule, it's easy to forget to run the script when updating the JSON files.
It's a follow-up on #9656 and #9951.