-
Notifications
You must be signed in to change notification settings - Fork 4
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
Ideas for metadata fields for reproducible builds #8
Comments
a CRC32 on library level can cause problems regarding configuration of libraries in their .h files |
@SirSydom |
some libraries contain defines in their header files to adapt the library to specific needs. some example I found after quick search: https://github.com/adafruit/Adafruit_NeoPixel/blob/13d4e20f60a7c3a909693759a01a84c781f732b1/Adafruit_NeoPixel.h#L134 This is a general design flaw in the arduino environment - because the library is for ALL you projects (sketches) but the change to its defines may be only valid for one specific sketch (or even on specific version of one specific sketch). To adress this, one could think about not touching the lib but specifing the defines to the preprocessor directly (which could be done in the propossed idea) - but this will only work if all relevant defines are made to work with that, e.g. with #ifndef FOO #define FOO BAR #endif So, guarding libraries you use untouched will work with a crc32. But what if the library has to be modified ? |
I see the point made and yes the CRC would produce a mismatch If the programmer knows it did change the library she/he should expect a CRC warning. The CRC should not be mandatory, but using it gives that little extra control. |
as I just recognized, there is a proposed solution for the problem: |
Sounds like a good generic solution - still, even from such modified library a CRC could be made :) |
We thought about adding a CRC, but in the end, we opted to not add it in this first iteration because it has some tricky issues to clear out:
BTW the idea of adding CRC is still valid IMHO, and we will surely add it in the future. |
@cmaglie Maybe it is possible to get a (weaker) signature from the cyclomatic complexity of the code?
Propose to add an appropriate label e.g. FUTURE to the issue. |
ideas to extend the yaml file with following fields
The CRC32 is the 'ultimate' compatibility check. OK other hashes might even be more 'ultimate' but you get the idea.
From the list above the CRC is the only one that can actually be used and affect the build process.
So the other ones may appear as comments?
The text was updated successfully, but these errors were encountered: