Hello Paul, thanks for improving the patch!
> * Would it be better to have --enable-dfltc be the default on develoment > platforms that have the support? The current plan is to enable this on a per-distro basis. > * Why was the change to tests/znew-k needed? If I understand correctly, znew -K compares whether the new file is smaller than the old one, and keeps the old one if this is not the case. The znew-k test expects that the replacement will happen, however, when DFLTCC is used, it does not. The description of the test states: "Check that znew -K works without compress(1)“, so I assume whether or not the file is replaced is irrelevant w.r.t. the main point of the test. Therefore I changed it to accept both outcomes. > * What is the significance of the SOURCE_DATE_EPOCH environment > variable? Should that getenv be removed? Unfortunately DFLTCC can produce different (but valid!) outputs for the same input when called multiple times. This may pose a problem for reproducible builds, so ideally when gzip is used for building software, it should use software compression. SOURCE_DATE_EPOCH is an environment variable that instructs build tools to use a given timestamp instead of the current time [1] and I think nowadays it’s reasonable to expect that whenever reproducibility is expected, then this variable will be set. For example, Python looks at it when building the bytecode [2]. > * Should the other new environment variables be documented? Yes, I think so. Should this go to gzip.1 file? > * For new code it's better to use GNU style and assume now-ubiquitous > C99 constructs so I installed the attached further patch to do that. I > haven't tested it since I don't have access to the relevant hardware; > please give it a try when you have the chance. I have verified that the test suite passes with your changes. > * Anonymous structures and unions are a C11 feature. Currently we are > assuming only C99, so the attached patch redoes 'struct context' to omit > the anonymous union. > > * It's more portable to use C11-style 'alignas (8)' instead of > GCC-specific '__attribute__ ((aligned (8)))'. The Gnulib stdalign module > supports this under C99. I gave that a whirl in the attached patch. > > * Should have a NEWS entry; I added one in the attached patch. > > * I didn't understand why that '(void)blocksize;' needed to be there, so > I removed it; if it's needed please explain. This must be a leftover that I have missed when cleaning up the code. [1] https://reproducible-builds.org/docs/source-date-epoch/ [2] https://docs.python.org/3/library/py_compile.html#py_compile.compile Best regards, Ilya
