This is likely because of #43953 -> timestamp of the generated files is not updated, so it remains older than that of source files, no matter how many times generator is run.
One (quick and dirty) way to solve the potential problem for release tarballs would be to run the script with --force-overwrite once, to make sure the generated files are newer. For this, it might be sensible to make some of generate_packets.py's parameters more directly controllable through configure options. It might even be sensible to make force-overwrite the default, and only enable lazy-overwrite with certain configure options.
Another (more general) solution to prevent re-running the script every time, even with lazy-overwrite (which could become a problem anyway if the script ever starts taking longer to run), would be to take the intermediate dummy packets_generate file (which is currently temporarily created to guard against problems in parallel builds) and make it a permanent resident to use as a timestamp for when generate_packets.py was last run – it would always get refreshed by the build system, even when the actual output files didn't change.
Also, for the potential release tarball problem – maybe there could be a --without-python configure option (that must, of course, check that all generated files are already present) to generally sidestep that problem, rather than just trusting that the build system won't be running any Python scripts?
Reply To alienvalkyrie
Another (more general) solution to prevent re-running the script every time, even with lazy-overwrite (which could become a problem anyway if the script ever starts taking longer to run), would be to take the intermediate dummy packets_generate file (which is currently temporarily created to guard against problems in parallel builds) and make it a permanent resident to use as a timestamp for when generate_packets.py was last run – it would always get refreshed by the build system, even when the actual output files didn't change.
For that to work, it should also be created to the source directory. Which in itself might even be an improvement (that in case of multiple build directories, they all look at the same 'packets_generate')
But my initial thought was option (3) that even with lazy-overwrite we would still refresh the timestamp (touch the generated files).
Reply To cazfi
But my initial thought was option (3) that even with lazy-overwrite we would still refresh the timestamp (touch the generated files).
At that point, lazy-overwrite would do essentially nothing (except leave the originals untouched when there's an error in the generation process, in which case the build fails anyway, so it barely matters). The whole point of lazy-overwrite – at least for me – is that the generated files, particularly the headers, don't get touched, to avoid having to rebuild approximately the entire codebase. Especially right now, when I'm refactoring the generation script (which should leave the generated files unchanged), that saves a lot of time.
Reply To alienvalkyrie
Especially right now, when I'm refactoring the generation script (which should leave the generated files unchanged), that saves a lot of time.
If we consider that to be the special case -> one where one needs to explicitly set (non-default) options, how about:
1) Make forced overwrite the default in generate_packets.py, requiring extra option ("--lazy-overwrite") to change
2) Add support to common/Makefile (and meson.build?) for user to pass *additional* parameters to generate_packets.py, likely via environment variables
I do think it might be sensible to have lazy-overwrite enabled by default – not rebuilding stuff that hasn't actually changed is reasonable IMO. And since the various refactoring changes will end up in master one by one, this doesn't only affect people working on the generation scripts in particular, but anyone regularly building from master.
I also do think, whether lazy-overwrite is the default or optional, it'd be sensible to have an unambiguous indicator that current generated files are up to date, regardless of their timestamp (e.g. in the form of a dummy file). (Note that the same can be said for utility/generate_specenum.py and any other future code generation scripts that might get added.)
Reply To cazfi
2) Add support to common/Makefile (and meson.build?) for user to pass *additional* parameters to generate_packets.py, likely via environment variables
While looking at this (and trying to understand how autoconf works), I just noticed configure.ac already does AC_SUBST([GENERATE_PACKETS_ARGS]) – the variable just isn't used anywhere yet.
Reply To cazfi
1) Make forced overwrite the default in generate_packets.py, requiring extra option ("--lazy-overwrite") to change
2) Add support to common/Makefile (and meson.build?) for user to pass *additional* parameters to generate_packets.py, likely via environment variables
Attached patch does exactly that (minus the meson.build part). Part of me wants to split it into two even tinier patches, since they aren't really dependent on each other.
The GENERATE_PACKETS_ARGS variable isn't documented in ./configure --help – should we do that? If I understand autoconf correctly, that would mean replacing the AC_SUBST with an AC_ARG_VAR macro (plus description)?
Reply To alienvalkyrie
Reply To cazfi
1) Make forced overwrite the default in generate_packets.py, requiring extra option ("--lazy-overwrite") to change
2) Add support to common/Makefile (and meson.build?) for user to pass *additional* parameters to generate_packets.py, likely via environment variables
Attached patch does exactly that (minus the meson.build part). Part of me wants to split it into two even tinier patches, since they aren't really dependent on each other.
The GENERATE_PACKETS_ARGS variable isn't documented in ./configure --help – should we do that? If I understand autoconf correctly, that would mean replacing the AC_SUBST with an AC_ARG_VAR macro (plus description)?
New patch includes documentation for autotools build, and support for additional packet gen arguments in meson.build – the latter is completely untested (except that it didn't break the CI build).
While "disable lazy-overwrite unless specifically requested" isn't my preferred way or resolving this issue (which would've been adding dummy files as unambiguous indicators of last regeneration), I think it's reasonable enough (and simpler).
(There is still an argument to be made to split this into "pass through additional arguments" and "disable lazy-overwrite by default".)
Current master: even when I run 'make' twice in a row, without changing absolutely anything, the output shows that generate_packets.py gets run. Don't understand why. I've checked timestamps of both generate_packets.py and packets.def, and neither has any weird in-future timestamp.
This is probably mostly harmless in a development branch, but if the same happens (haven't tested) in a build from a release tarball, that breaks a build without python.