This file is auto-generated by CMake from WasmCommon.h.in, I am not sure whether to track it or not.

The usual practice for auto-generated files is to generate them in the build directory, and never modify the input source tree. It's especially useful when you set up multi-config builds which might need different versions of the same headers. I think faasm already breaks this rule with the current protobuf configuration (I did change that in my fork at some point), but it might be good to follow for future changes

Great, thanks for chipping in, was about to ask you at some point.

We could eventually change the protobuf headers as well.

Follow-up, I realised FAASM_INCLUDE_DIR points to CMAKE_LIST_DIR/include, should we eventually move to CMAKE_BINARY_DIR/include and copy the includes there?

I will now do it just for one file, but for the protobuf change for example.

Also, whilst we are at it, I personally prefer to have the project name at the begining of the path, so:

#include <faasm/wasm/WasmCommon.h>

so we could copy CMAKE_LIST_DIR/include into CMAKE_BINARY_DIR/include/faasm/include what are your thoughts?

I think both should be there: CMAKE_LIST_DIR/include first, and then CMAKE_BUILD_DIR/include second. That way the main source tree files take precedence, but you still get access to generated headers. It also means you can theoretically alias names and use #include_next to get an identically named header from the build tree if you want insert a hook/wrap the generated header in some extra code

@eigenraven requesting a review from you as well see if i have overlooked something

Is there a point to keeping bytes-per-page dynamic? Afaik this value is frozen at 64k in the wasm spec

Do you have any particular reason to grab these from the environment? Usually you'd use cache variables for these sort of tunables, done with set(... CACHE type "docstring"), as they are less fragile if you accidentally call cmake from the wrong context

The goal of this PR is to share this constant definitions with the CPP toolchain, and have them hard-coded only once across all the repos. IMO, even if they won't change, I'd rather have them hardcoded only once (if that doesn't add too much overhead). They are all hardcoded in faasm/cpp/faasmtools/build.py.

So, the way I thought of sharing variables between a python script (faasm/cpp/faasmtools/build.py) and CMake files was through environment variables. It feels a bit hacky, so if you have another alternative I'm happy to adopt it.

If they're in a python dict, you should be able to also just pass them in to cmake during configuration time via the -D flags when they're cache variables

@eigenraven is this more or less what you had in mind? Also, is there any cleaner way to check if any is not set?

Yes, and the conditions can probably just be if((NOT FAASM_WASM_BYTES_PER_PAGE) OR (NOT FAASM_WASM_CTORS_FUNC_NAME) ) etc. What you wrote is wrong, because you force expansion with ${}, so it would evaluate to NOT DEFINED CACHE{65536} - when passing variables "by reference" to cmake commands you always name them, without quotes or $-expansion.