Commit 9352f6f85f04f90a193f854bd39b31dec9913794
Committed by
GitHub
Merge pull request #1670 from m-holger/rm_maint
Split `README-maintainer.md` into `README-maintainer.md` and `README-developer.md`
Showing
15 changed files
with
712 additions
and
687 deletions
README-developer.md
0 → 100644
| 1 | +# Notes for Contributors and Other qpdf Developers | ||
| 2 | + | ||
| 3 | +This file contains notes of interest to developers that want to modify qpdf itself rather than using | ||
| 4 | +qpdf as a library. | ||
| 5 | + | ||
| 6 | +## Contents | ||
| 7 | + | ||
| 8 | +* [ROUTINE DEVELOPMENT](#routine-development) | ||
| 9 | +* [CHECKING DOCS ON readthedocs](#checking-docs-on-readthedocs) | ||
| 10 | +* [CODING RULES](#coding-rules) | ||
| 11 | +* [ZLIB COMPATIBILITY](#zlib-compatibility) | ||
| 12 | +* [HOW TO ADD A COMMAND-LINE ARGUMENT](#how-to-add-a-command-line-argument) | ||
| 13 | +* [RUNNING pikepdf's TEST SUITE](#running-pikepdfs-test-suite) | ||
| 14 | +* [OTHER NOTES](#other-notes) | ||
| 15 | +* [DEPRECATION](#deprecation) | ||
| 16 | +* [LOCAL WINDOWS TESTING PROCEDURE](#local-windows-testing-procedure) | ||
| 17 | +* [DOCS ON readthedocs.org](#docs-on-readthedocsorg) | ||
| 18 | +* [CMAKE notes](#cmake-notes) | ||
| 19 | +* [ABI checks](#abi-checks) | ||
| 20 | +* [CODE FORMATTING](#code-formatting) | ||
| 21 | + | ||
| 22 | + | ||
| 23 | +## ROUTINE DEVELOPMENT | ||
| 24 | + | ||
| 25 | +**When making changes that users need to know about, update the release notes | ||
| 26 | +(manual/release-notes.rst) as you go.** Major changes to the internal API can also be mentioned in | ||
| 27 | +the release notes in a section called "Internal Changes" or similar. This removes ChangeLog as a | ||
| 28 | +separate mechanism for tracking changes. | ||
| 29 | + | ||
| 30 | +**Remember to check pull requests as well as issues in github.** | ||
| 31 | + | ||
| 32 | +Run `cmake --list-presets` to see available cmake presets. Routine maintainer development can be | ||
| 33 | + | ||
| 34 | +``` | ||
| 35 | +cmake --preset maintainer | ||
| 36 | +cmake --build --preset maintainer | ||
| 37 | +ctest --preset maintainer | ||
| 38 | +``` | ||
| 39 | + | ||
| 40 | +See [CMakePresets.json](CMakePresets.json) for additional presets. Reminders about presets: | ||
| 41 | +* You can override/enhance configure presets, e.g., `cmake --preset maintainer -DCMAKE_BUILD_TYPE=Release` | ||
| 42 | +* You can pass flags to ctest, e.g., `ctest --preset maintainer -R zlib-flate`. | ||
| 43 | +* You can't override the build directory for build and test presets, but you _can_ override the | ||
| 44 | + directory for configure presets and then run `cmake --build build-dir` and `ctest` manually, as | ||
| 45 | + shown below. | ||
| 46 | +* The base configuration includes `-DCMAKE_EXPORT_COMPILE_COMMANDS=ON`, which is useful for LSP mode | ||
| 47 | + in C++. This is harmless in environments where it's not needed. You may need to make a symlink | ||
| 48 | + from compile_commands.json to the one in whichever build directory you are using. | ||
| 49 | +* If you have common configurations you'd like to see, pull requests are welcome, but | ||
| 50 | + `CMakeUserPresets.json` is your friend. You can copy or inherit from CMakeUserPresets.json for | ||
| 51 | + your own use. Note that CMakeUserPresets.json is not part of the stable API. We reserve the right | ||
| 52 | + to modify these presets in a non-compatible fashion at any time without regard to qpdf version | ||
| 53 | + numbers, but we should mention changes in the release notes. | ||
| 54 | +* Study the CMakePresets.json file for details on how these are implemented. | ||
| 55 | + | ||
| 56 | +See also ./build-scripts for other ways to run the build for different configurations. | ||
| 57 | + | ||
| 58 | +### Useful build examples | ||
| 59 | + | ||
| 60 | +To run a maintainer build in release mode and run only the unicode-filenames test, you could run | ||
| 61 | + | ||
| 62 | +``` | ||
| 63 | +cmake --preset maintainer -DCMAKE_BUILD_TYPE=Release | ||
| 64 | +cmake --build --preset maintainer | ||
| 65 | +TESTS=unicode-filenames ctest --preset maintainer -R qpdf | ||
| 66 | +``` | ||
| 67 | + | ||
| 68 | +To run a maintainer build in release mode in a _different directory_ and run only the | ||
| 69 | +unicode-filenames test, you could run the following. Trying to override the directory on the command | ||
| 70 | +line of `cmake --build` or `ctest` in conjunction with `--preset` may silently ignore the directory | ||
| 71 | +override, and you may not get what you think you are getting. | ||
| 72 | + | ||
| 73 | +``` | ||
| 74 | +cmake --preset maintainer -DCMAKE_BUILD_TYPE=Release -B cmake-build-release | ||
| 75 | +cmake --build cmake-build-release | ||
| 76 | +TESTS=unicode-filenames ctest --verbose --test-dir cmake-build-release -R qpdf | ||
| 77 | +``` | ||
| 78 | + | ||
| 79 | +### Profiling | ||
| 80 | + | ||
| 81 | +When running with the `maintainer-profile` preset (or any time you run profiling), run `gprof | ||
| 82 | +gmon.out`. Note that gmon.out is not cumulative. | ||
| 83 | + | ||
| 84 | +### Coverage | ||
| 85 | + | ||
| 86 | +When running with the `maintainer-coverage` preset, after running tests: | ||
| 87 | +``` | ||
| 88 | +gcovr -r .. --html --html-details -o coverage-report.html | ||
| 89 | +``` | ||
| 90 | + | ||
| 91 | +Note that, in early 2024, branch coverage information is not very accurate with C++. | ||
| 92 | + | ||
| 93 | +### Sanitizers/Memory Checks | ||
| 94 | + | ||
| 95 | +If `clang++` fails to create output during configuration, it may be necessary to install a specific | ||
| 96 | +version of libstdc++-dev. For example, with clang++ version 20 on Ubuntu 24.04, `clang++ -v` | ||
| 97 | +indicates the selected GCC installation is 14, so it is necessary to install `libstdc++-14-dev`. | ||
| 98 | + | ||
| 99 | +### Windows | ||
| 100 | + | ||
| 101 | +You can use this for command-line builds, which does a bit more than the presets. The msvc presets | ||
| 102 | +are known to work in CLion if the environment is set up as described in | ||
| 103 | +[README-windows.md](./README-windows.md), but for regular command-line builds (and CI), continue to | ||
| 104 | +use `cmake-win` from inside a build directory. Look at `build-scripts/build-windows` to see how this | ||
| 105 | +is used. | ||
| 106 | + | ||
| 107 | +``` | ||
| 108 | +../cmake-win {mingw|msvc} maint | ||
| 109 | +``` | ||
| 110 | + | ||
| 111 | +## CHECKING DOCS ON readthedocs | ||
| 112 | + | ||
| 113 | +To check docs on readthedocs.io without running all of CI, push to the | ||
| 114 | +doc-check branch. Then visit https://qpdf.readthedocs.io/en/doc-check/ | ||
| 115 | +Building docs from pull requests is also enabled. | ||
| 116 | + | ||
| 117 | +## CODING RULES | ||
| 118 | + | ||
| 119 | +* Code is formatted with clang-format. See .clang-format and the | ||
| 120 | + "Code Formatting" section in manual/contributing.rst for details. | ||
| 121 | + See also "CODE FORMATTING" below. | ||
| 122 | + | ||
| 123 | +* Use std::to_string instead of QUtil::int_to_string et al | ||
| 124 | + | ||
| 125 | +* Use of assert: | ||
| 126 | + | ||
| 127 | + * Test code: #include <qpdf/assert_test.h> first. | ||
| 128 | + * Debug code: #include <qpdf/assert_debug.h> first and use | ||
| 129 | + qpdf_assert_debug instead of assert. Note that <qpdf/Util.hh> | ||
| 130 | + includes assert_debug.h. Include this instead if 'At most one | ||
| 131 | + qpdf/assert header ...' errors are encountered, especially when | ||
| 132 | + using assert in private header files. | ||
| 133 | + * Use 'qpdf_expect', 'qpdf_static_expect', 'qpdf_ensures' and | ||
| 134 | + 'qpdf_invariant' to document pre/post-conditions and invariants. | ||
| 135 | + This requires inclusion of 'assert_debug.h' or 'Util.hh'. Remember | ||
| 136 | + that these (except for 'qpdf_static_expect') are only checked in | ||
| 137 | + debug builds. | ||
| 138 | + * Use 'util::assertion' when checks should also be carried out in | ||
| 139 | + release code in preference to throwing logic_errors directly | ||
| 140 | + unless it is practical and desirable to test violations during | ||
| 141 | + CI testing. This avoids obscuring genuine gaps in coverage with | ||
| 142 | + noise generated by unreachable sanity checks. | ||
| 143 | + | ||
| 144 | + These rules are enforced by the check-assert test. This practices | ||
| 145 | + serves to | ||
| 146 | + | ||
| 147 | + * remind us that assert in release code disappears and so should only | ||
| 148 | + be used to document pre/post conditions and invariants, and for | ||
| 149 | + sanity checks during development and testing; when doing so use | ||
| 150 | + a Debug build configuration. | ||
| 151 | + | ||
| 152 | + * protect us from using assert in test code without explicitly | ||
| 153 | + removing the NDEBUG definition, since that would cause the assert | ||
| 154 | + not to actually be testing anything in non-Debug build | ||
| 155 | + configurations. | ||
| 156 | + | ||
| 157 | + Prior to 12.3 assert_test.h and assert_debug.h shared the same header | ||
| 158 | + guard, which prevented the simultaneous inclusion of both headers. | ||
| 159 | + This was changed to permit the CI testing of private-API methods | ||
| 160 | + without loosing the use of assertions in private header files. | ||
| 161 | + | ||
| 162 | +* In a source file, include the header file that declares the source | ||
| 163 | + class first followed by a blank line. If a config file is needed | ||
| 164 | + first, put a blank line between that and the header followed by | ||
| 165 | + another blank line. This assures that each header file is included | ||
| 166 | + first at least once, thereby ensuring that it explicitly includes | ||
| 167 | + all the headers it needs, which in turn alleviates lots of header | ||
| 168 | + ordering problems. The blank line ensures that formatters don't | ||
| 169 | + mess this up by resorting the headers. | ||
| 170 | + | ||
| 171 | +* Avoid atoi. Use QUtil::string_to_int instead. It does | ||
| 172 | + overflow/underflow checking. | ||
| 173 | + | ||
| 174 | +* Avoid certain functions that tend to be macros or create compilation | ||
| 175 | + errors on some platforms. Known cases: strcasecmp, abs. Avoid min | ||
| 176 | + and max. If needed, std::min and std::max are okay to use in C++ | ||
| 177 | + code with <algorithm> included. | ||
| 178 | + | ||
| 179 | +* Remember to avoid using `operator[]` with `std::string` or | ||
| 180 | + `std::vector`. Instead, use `at()`. See README-hardening.md for | ||
| 181 | + details. | ||
| 182 | + | ||
| 183 | +* Use QIntC for type conversions -- see casting policy in docs. | ||
| 184 | + | ||
| 185 | +* Remember to imbue ostringstreams with std::locale::classic() before | ||
| 186 | + outputting numbers. This protects against the user's global locale | ||
| 187 | + altering otherwise deterministic values. (See github issue #459.) | ||
| 188 | + One could argue that error messages containing numbers should | ||
| 189 | + respect the user's locale, but I think it's more important for | ||
| 190 | + output to be consistent, since the messages in question are not | ||
| 191 | + really targeted at the end user. | ||
| 192 | + | ||
| 193 | +* Use QPDF_DLL on all methods that are to be exported in the shared | ||
| 194 | + library/DLL. Use QPDF_DLL_CLASS for all classes whose type | ||
| 195 | + information is needed. This is important for classes that are used | ||
| 196 | + as exceptions, subclassed, or tested with dynamic_cast across the | ||
| 197 | + the shared object boundary (or "shared library boundary" -- we may | ||
| 198 | + use either term in comments and documentation). In particular, | ||
| 199 | + anything new derived from Pipeline or InputSource should be marked | ||
| 200 | + with QPDF_DLL_CLASS. We shouldn't need to do it for QPDFObjectHelper | ||
| 201 | + or QPDFDocumentHelper subclasses since there's no reason to use | ||
| 202 | + dynamic_cast with those, but doing it anyway may help with some | ||
| 203 | + strange cases for mingw or with some code generators that may | ||
| 204 | + systematically do this for other reasons. | ||
| 205 | + | ||
| 206 | + IMPORTANT NOTE ABOUT QPDF_DLL_CLASS: On mingw, the vtable for a | ||
| 207 | + class with some virtual methods and no pure virtual methods seems | ||
| 208 | + often (always?) not to be generated if the destructor is inline or | ||
| 209 | + declared with `= default`. Therefore, for any class that is intended | ||
| 210 | + to be used as a base class and doesn't contain any pure virtual | ||
| 211 | + methods, you must declare the destructor in the header without | ||
| 212 | + `= default` and provide a non-inline implementation in the source | ||
| 213 | + file. Add this comment to the implementation: | ||
| 214 | + | ||
| 215 | + ```cpp | ||
| 216 | + // Must be explicit and not inline -- see QPDF_DLL_CLASS in | ||
| 217 | + // README-maintainer | ||
| 218 | + ``` | ||
| 219 | + | ||
| 220 | +* Put private member variables in std::unique_ptr<Members> for all | ||
| 221 | + public classes. Forward declare Members in the header file and define | ||
| 222 | + Members in the implementation file. One of the major benefits of | ||
| 223 | + defining Members in the implementation file is that it makes it easier | ||
| 224 | + to use private classes as data members and simplifies the include order. | ||
| 225 | + Remember that Members must be fully defined before the destructor of the | ||
| 226 | + main class. For an example of this pattern see class JSONHandler. | ||
| 227 | + | ||
| 228 | + Exception: indirection through std::unique_ptr<Members> incurs an overhead, | ||
| 229 | + so don't do it for: | ||
| 230 | + * (especially private) classes that are copied a lot, like QPDFObjectHandle | ||
| 231 | + and QPDFObject. | ||
| 232 | + * classes that are a shared pointer to another class, such as QPDFObjectHandle | ||
| 233 | + or JSON. | ||
| 234 | + | ||
| 235 | + For exported classes that do not use the member pattern for performance | ||
| 236 | + reasons it is worth considering adding a std::unique_ptr to an empty Members | ||
| 237 | + class initialized to nullptr to give the flexibility to add data members | ||
| 238 | + without breaking the ABI. | ||
| 239 | + | ||
| 240 | + Note that, as of qpdf 11, many public classes use `std::shared_ptr` | ||
| 241 | + instead. Changing this to `std::unique_ptr` is ABI-breaking. If the | ||
| 242 | + class doesn't allow copying, we can switch it to std::unique_ptr and | ||
| 243 | + let that be the thing that prevents copying. If the intention is to | ||
| 244 | + allow the object to be copied by value and treated as if it were | ||
| 245 | + copied by reference, then `std::shared_ptr<Members>` should be used. | ||
| 246 | + The `JSON` class is an example of this. As a rule, we should avoid | ||
| 247 | + this design pattern. It's better to make things non-copiable and to | ||
| 248 | + require explicit use of shared pointers, so going forward, | ||
| 249 | + `std::unique_ptr` should be preferred. | ||
| 250 | + | ||
| 251 | +* Traversal of objects is expensive. It's worth adding some complexity | ||
| 252 | + to avoid needless traversals of objects. | ||
| 253 | + | ||
| 254 | +* Avoid attaching too much metadata to objects and object handles | ||
| 255 | + since those have to get copied around a lot. | ||
| 256 | + | ||
| 257 | +* Prefer std::string_view to std::string const& and char const*. | ||
| 258 | + | ||
| 259 | + * Where functions rely on strings being null-terminated, std::string_view may not be appropriate. | ||
| 260 | + | ||
| 261 | + * For return values, consider whether returning a string_view is safe or whether it is more appropriate | ||
| 262 | + to return a std::string or std::string const&, especially in the public API. | ||
| 263 | + | ||
| 264 | + * NEVER replace a std::string const& return value with std::string_view in the public API. | ||
| 265 | + | ||
| 266 | + | ||
| 267 | +## ZLIB COMPATIBILITY | ||
| 268 | + | ||
| 269 | +The qpdf test suite is designed to be independent of the output of any | ||
| 270 | +particular version of zlib. (See also `ZOPFLI` in README.md.) There | ||
| 271 | +are several strategies to make this work: | ||
| 272 | + | ||
| 273 | +* `build-scripts/test-alt-zlib` runs in CI and runs the test suite | ||
| 274 | + with a non-default zlib. Please refer to that code for an example of | ||
| 275 | + how to do this in case you want to test locally. | ||
| 276 | + | ||
| 277 | +* The test suite is full of cases that compare output PDF files with | ||
| 278 | + expected PDF files in the test suite. If the file contains data that | ||
| 279 | + was compressed by QPDFWriter, then the output file will depend on | ||
| 280 | + the behavior of zlib. As such, using a simple comparison won't work. | ||
| 281 | + There are several strategies used by the test suite. | ||
| 282 | + | ||
| 283 | + * A new program called `qpdf-test-compare`, in most cases, is a drop | ||
| 284 | + in replacement for a simple file comparison. This code make sure | ||
| 285 | + the two files have exactly the same number of objects with the | ||
| 286 | + same object and generation numbers, and that corresponding objects | ||
| 287 | + are identical with the following allowances (consult its source | ||
| 288 | + code for all the details details): | ||
| 289 | + * The `/Length` key is not compared in stream dictionaries. | ||
| 290 | + * The second element of `/ID` is not compared. | ||
| 291 | + * If the first and second element of `/ID` are the same, then the | ||
| 292 | + first element if `/ID` is also not compared. | ||
| 293 | + * If a stream is compressed with `/FlateDecode`, the | ||
| 294 | + _uncompressed_ stream data is compared. Otherwise, the raw | ||
| 295 | + stream data is compared. | ||
| 296 | + * Generated fields in the `/Encrypt` dictionary are not compared, | ||
| 297 | + though password-protected files must have the same password. | ||
| 298 | + * Differences in the contents of `/XRef` streams are ignored. | ||
| 299 | + | ||
| 300 | + To use this, run `qpdf-test-compare actual.pdf expected.pdf`, and | ||
| 301 | + expect the output to match `expected.pdf`. For example, if a test | ||
| 302 | + used to be written like this; | ||
| 303 | + ```perl | ||
| 304 | + $td->runtest("check output", | ||
| 305 | + {$td->FILE => "a.pdf"}, | ||
| 306 | + {$td->FILE => "out.pdf"}); | ||
| 307 | + ``` | ||
| 308 | + then write it like this instead: | ||
| 309 | + ```perl | ||
| 310 | + $td->runtest("check output", | ||
| 311 | + {$td->COMMAND => "qpdf-test-compare a.pdf out.pdf"}, | ||
| 312 | + {$td->FILE => "out.pdf", $td->EXIT_STATUS => 0}); | ||
| 313 | + ``` | ||
| 314 | + You can look at `compare-for-test/qtest/compare.test` for | ||
| 315 | + additional examples. | ||
| 316 | + | ||
| 317 | + Here's what's going on: | ||
| 318 | + * If the files "match" according to the rules of | ||
| 319 | + `qpdf-test-compare`, the output of the program is the expected | ||
| 320 | + file. | ||
| 321 | + * If the files do not match, the output is the actual file. The | ||
| 322 | + reason is that, if a change is made that results in an expected | ||
| 323 | + change to the expected file, the output of the comparison can be | ||
| 324 | + used to replace the expected file (as long as it is definitely | ||
| 325 | + known to be correct—no shortcuts here!). That way, it doesn't | ||
| 326 | + matter which zlib you use to generate test files. | ||
| 327 | + * As a special debugging tool, you can set the `QPDF_COMPARE_WHY` | ||
| 328 | + environment variable to any value. In this case, if the files | ||
| 329 | + don't match, the output is a description of the first thing in | ||
| 330 | + the file that doesn't match. This is mostly useful for debugging | ||
| 331 | + `qpdf-test-compare` itself, but it can also be helpful as a | ||
| 332 | + sanity check that the differences are expected. If you are | ||
| 333 | + trying to find out the _real_ differences, a suggestion is to | ||
| 334 | + convert both files to qdf and compare them lexically. | ||
| 335 | + | ||
| 336 | + * There are some cases where `qpdf-test-compare` can't be used. For | ||
| 337 | + example, if you need to actually test one of the things that | ||
| 338 | + `qpdf-test-compare` ignores, you'll need some other mechanism. | ||
| 339 | + There are tests for deterministic ID creation and xref streams | ||
| 340 | + that have to implement other mechanisms. Also, linearization hint | ||
| 341 | + streams and the linearization dictionary in a linearized file | ||
| 342 | + contain file offsets. Rather than ignoring those, it can be | ||
| 343 | + helpful to create linearized files using `--compress-streams=n`. | ||
| 344 | + In that case, `QPDFWriter` won't compress any data, so the PDF | ||
| 345 | + will be independent of the output of any particular zlib | ||
| 346 | + implementation. | ||
| 347 | + | ||
| 348 | +You can find many examples of how tests were rewritten by looking at | ||
| 349 | +the commits preceding the one that added this section of this README | ||
| 350 | +file. | ||
| 351 | + | ||
| 352 | +Note about `/ID`: many test cases use `--static-id` to have a | ||
| 353 | +predictable `/ID` for testing. Many other test cases use | ||
| 354 | +`--deterministic-id`. While `--static-id` is unaffected by file | ||
| 355 | +contents, `--deterministic-id` is based on file contents and so is | ||
| 356 | +dependent on zlib output if there is any newly compressed data. By | ||
| 357 | +using `qpdf-test-compare`, it's actually not necessary to use either | ||
| 358 | +`--static-id` or `--deterministic-id`. It may still be necessary to | ||
| 359 | +use `--static-aes-iv` if comparing encrypted files, but since | ||
| 360 | +`qpdf-test-compare` ignores `/Perms`, a wider range of encrypted files | ||
| 361 | +can be compared using `qpdf-test-compare`. | ||
| 362 | + | ||
| 363 | + | ||
| 364 | +## HOW TO ADD A COMMAND-LINE ARGUMENT | ||
| 365 | + | ||
| 366 | +Quick reminder: | ||
| 367 | + | ||
| 368 | +* Add an entry to the top half of job.yml for the command-line | ||
| 369 | + argument | ||
| 370 | +* Add an entry to the bottom half of job.yml for the job JSON field | ||
| 371 | +* Add documentation for the new option to cli.rst | ||
| 372 | +* Implement the QPDFJob::Config method in QPDFJob_config.cc | ||
| 373 | +* Pass the build option `-DGENERATE_AUTO_JOB=1` to `cmake` | ||
| 374 | + (see [here](https://qpdf.readthedocs.io/en/stable/installation.html#options-for-working-on-qpdf)) | ||
| 375 | + or run `generate_auto_job` manually. | ||
| 376 | +* Adding new options tables is harder -- see below | ||
| 377 | + | ||
| 378 | +QPDFJob is documented in three places: | ||
| 379 | + | ||
| 380 | +* This section provides a quick reminder for how to add a command-line | ||
| 381 | + argument | ||
| 382 | + | ||
| 383 | +* generate_auto_job has a detailed explanation about how QPDFJob and | ||
| 384 | + generate_auto_job work together | ||
| 385 | + | ||
| 386 | +* The manual ("QPDFJob Design" in qpdf-job.rst) discusses the design | ||
| 387 | + approach, rationale, and evolution of QPDFJob. | ||
| 388 | + | ||
| 389 | +Command-line arguments are closely coupled with QPDFJob. To add a new | ||
| 390 | +command-line argument, add the option to the appropriate table in | ||
| 391 | +job.yml. After `generate_auto_job` is run (either manually or as part | ||
| 392 | +of the build process, when `GENERATE_AUTO_JOB` is set), this will | ||
| 393 | +automatically declare a method in the private ArgParser class in | ||
| 394 | +QPDFJob_argv.cc which you have to implement. The implementation should | ||
| 395 | +make calls to methods in QPDFJob via its Config classes. Then, add the | ||
| 396 | +same option to either the no-json section of job.yml if it is to be | ||
| 397 | +excluded from the job json structure, or add it under the json | ||
| 398 | +structure to the place where it should appear in the json structure. | ||
| 399 | + | ||
| 400 | +In most cases, adding a new option will automatically declare and call | ||
| 401 | +the appropriate Config method, which you then have to implement. If | ||
| 402 | +you need a manual handler, you have to declare the option as manual in | ||
| 403 | +job.yml and implement the handler yourself, though the automatically | ||
| 404 | +generated code will declare it for you. | ||
| 405 | + | ||
| 406 | +Adding a new option table is a bit harder and is not well-documented. | ||
| 407 | +For a simple example, look at the code that added the | ||
| 408 | +--set-page-labels table. That change was divided into two commits (one | ||
| 409 | +for the manual changes, and one for the generated changes) to make it | ||
| 410 | +easier to use as an example. | ||
| 411 | + | ||
| 412 | +The build will fail until the new option is documented in | ||
| 413 | +manual/cli.rst. To do that, create documentation for the option by | ||
| 414 | +adding a ".. qpdf:option::" directive followed by a magic help comment | ||
| 415 | +as described at the top of manual/cli.rst. Put this in the correct | ||
| 416 | +help topic. Help topics roughly correspond with sections in that | ||
| 417 | +chapter and are created using a special ".. help-topic" comment. | ||
| 418 | +Follow the example of other options for style. | ||
| 419 | + | ||
| 420 | +When done, the following should happen: | ||
| 421 | + | ||
| 422 | +* qpdf --new-option should work as expected | ||
| 423 | +* qpdf --help=--new-option should show the help from the comment in cli.rst | ||
| 424 | +* qpdf --help=topic should list --new-option for the correct topic | ||
| 425 | +* --new-option should appear in the manual | ||
| 426 | +* --new-option should be in the command-line option index in the manual | ||
| 427 | +* A Config method (in Config or one of the other Config classes in | ||
| 428 | + QPDFJob) should exist that corresponds to the command-line flag | ||
| 429 | +* The job JSON file should have a new key in the schema corresponding | ||
| 430 | + to the new option | ||
| 431 | + | ||
| 432 | + | ||
| 433 | +## RUNNING pikepdf's TEST SUITE | ||
| 434 | + | ||
| 435 | +We run pikepdf's test suite from CI. These instructions show how to do | ||
| 436 | +it manually. | ||
| 437 | + | ||
| 438 | +Do this in a separate shell. | ||
| 439 | + | ||
| 440 | +``` | ||
| 441 | +cd ...qpdf-source-tree... | ||
| 442 | +export QPDF_SOURCE_TREE=$PWD | ||
| 443 | +export QPDF_BUILD_LIBDIR=$QPDF_SOURCE_TREE/build/libqpdf | ||
| 444 | +export LD_LIBRARY_PATH=$QPDF_BUILD_LIBDIR | ||
| 445 | +rm -rf /tmp/z | ||
| 446 | +mkdir /tmp/z | ||
| 447 | +cd /tmp/z | ||
| 448 | +git clone git@github.com:pikepdf/pikepdf | ||
| 449 | +python3 -m venv v | ||
| 450 | +source v/bin/activate | ||
| 451 | +cd pikepdf | ||
| 452 | +python3 -m pip install --upgrade pip | ||
| 453 | +python3 -m pip install '.[test]' | ||
| 454 | +rehash | ||
| 455 | +python3 -m pip install . | ||
| 456 | +pytest -n auto | ||
| 457 | +``` | ||
| 458 | + | ||
| 459 | +If there are failures, use git bisect to figure out where the failure | ||
| 460 | +was introduced. For example, set up a work area like this: | ||
| 461 | + | ||
| 462 | +``` | ||
| 463 | +rm -rf /tmp/z | ||
| 464 | +mkdir /tmp/z | ||
| 465 | +cd /tmp/z | ||
| 466 | +git clone file://$HOME/source/qpdf/qpdf/.git qpdf | ||
| 467 | +git clone git@github.com:pikepdf/pikepdf | ||
| 468 | +export QPDF_SOURCE_TREE=/tmp/z/qpdf | ||
| 469 | +export QPDF_BUILD_LIBDIR=$QPDF_SOURCE_TREE/build/libqpdf | ||
| 470 | +export LD_LIBRARY_PATH=$QPDF_BUILD_LIBDIR | ||
| 471 | +cd qpdf | ||
| 472 | +mkdir build | ||
| 473 | +cmake -B build -DMAINTAINER_MODE=ON -DBUILD_STATIC_LIBS=OFF \ | ||
| 474 | + -DCMAKE_BUILD_TYPE=RelWithDebInfo | ||
| 475 | +cat <<'EOF' | ||
| 476 | +#!/bin/bash | ||
| 477 | +cd /tmp/z/pikepdf | ||
| 478 | +cmake --build /tmp/z/qpdf/build -j16 --target libqpdf -- -k | ||
| 479 | +git clean -dfx | ||
| 480 | +rm -rf ../v | ||
| 481 | +python3 -m venv ../v | ||
| 482 | +source ../v/bin/activate | ||
| 483 | +python3 -m pip install --upgrade pip | ||
| 484 | +python3 -m pip install '.[test]' | ||
| 485 | +python3 -m pip install . | ||
| 486 | +pytest -n auto | ||
| 487 | +EOF | ||
| 488 | +chmod +x /tmp/check | ||
| 489 | +``` | ||
| 490 | + | ||
| 491 | +Then in /tmp/z/qpdf, run git bisect. Use /tmp/check at each stage to | ||
| 492 | +test whether it's a good or bad commit. | ||
| 493 | + | ||
| 494 | + | ||
| 495 | +## OTHER NOTES | ||
| 496 | + | ||
| 497 | +For local iteration on the AppImage generation, it works to just | ||
| 498 | +./build-scripts/build-appimage and get the resulting AppImage from the | ||
| 499 | +distribution directory. You can pass additional arguments to | ||
| 500 | +build-appimage, which passes them along to to docker. | ||
| 501 | + | ||
| 502 | +Use -e SKIP_TESTS=1 to skip the test suite. | ||
| 503 | +Use -ti -e RUN_SHELL=1 to run a shell instead of the build script. | ||
| 504 | + | ||
| 505 | +To iterate on the scripts directly in the source tree, you can run | ||
| 506 | + | ||
| 507 | +``` | ||
| 508 | +docker build -t qpdfbuild appimage | ||
| 509 | +docker run --privileged --rm -ti -e SKIP_TESTS=1 -e RUN_SHELL=1 \ | ||
| 510 | + -v $PWD/..:/tmp/build ${1+"$@"} qpdfbuild | ||
| 511 | +``` | ||
| 512 | + | ||
| 513 | +This will put you at a shell prompt inside the container with your | ||
| 514 | +current directory set to the top of the source tree and your uid equal | ||
| 515 | +to the owner of the parent directory source tree. | ||
| 516 | + | ||
| 517 | +Note: this will leave some extra files (like .bash_history) in the | ||
| 518 | +parent directory of the source tree. You will want to clean those up. | ||
| 519 | + | ||
| 520 | + | ||
| 521 | +## DEPRECATION | ||
| 522 | + | ||
| 523 | +This is a reminder of how to use and test deprecation. | ||
| 524 | + | ||
| 525 | +To temporarily disable deprecation warnings for testing: | ||
| 526 | + | ||
| 527 | +```cpp | ||
| 528 | +#ifdef _MSC_VER | ||
| 529 | +# pragma warning(disable : 4996) | ||
| 530 | +#endif | ||
| 531 | +#if (defined(__GNUC__) || defined(__clang__)) | ||
| 532 | +# pragma GCC diagnostic push | ||
| 533 | +# pragma GCC diagnostic ignored "-Wdeprecated-declarations" | ||
| 534 | +#endif | ||
| 535 | + // Do deprecated thing here | ||
| 536 | +#if (defined(__GNUC__) || defined(__clang__)) | ||
| 537 | +# pragma GCC diagnostic pop | ||
| 538 | +#endif | ||
| 539 | +``` | ||
| 540 | + | ||
| 541 | +To declare something as deprecated: | ||
| 542 | + | ||
| 543 | +```cpp | ||
| 544 | +[[deprecated("explanation")]] | ||
| 545 | +``` | ||
| 546 | + | ||
| 547 | +## LOCAL WINDOWS TESTING PROCEDURE | ||
| 548 | + | ||
| 549 | +This is what I do for routine testing on Windows. | ||
| 550 | + | ||
| 551 | +* From Windows, git clone from my Linux clone, and unzip | ||
| 552 | + `external-libs`. | ||
| 553 | + | ||
| 554 | +* Start a command-line shell for x86_64 and x86 tools from Visual | ||
| 555 | + studio. From there, start C:\msys64\mingw64 twice and | ||
| 556 | + C:\msys64\mingw32 twice. | ||
| 557 | + | ||
| 558 | +* Create a build directory for each of the four permutations. Then, in | ||
| 559 | + each build directory, run `../cmake-win <tool> maint`. | ||
| 560 | + | ||
| 561 | +* Run `cmake --build . -j4`. For MSVC, add `--config Release` | ||
| 562 | + | ||
| 563 | +* Test with with msvc: `ctest --verbose -C Release` | ||
| 564 | + | ||
| 565 | +* Test with mingw: `ctest --verbose -C RelWithDebInfo` | ||
| 566 | + | ||
| 567 | +## DOCS ON readthedocs.org | ||
| 568 | + | ||
| 569 | +* Registered for an account at readthedocs.org with my github account | ||
| 570 | +* Project page: https://readthedocs.org/projects/qpdf/ | ||
| 571 | +* Docs: https://qpdf.readthedocs.io/ | ||
| 572 | +* Admin -> Settings | ||
| 573 | + * Set project home page | ||
| 574 | + * Advanced | ||
| 575 | + * Show version warning | ||
| 576 | + * Default version: stable | ||
| 577 | + * Email Notifications: set email address for build failures | ||
| 578 | + | ||
| 579 | +At this time, there is nothing in .github/workflows to support this. | ||
| 580 | +It's all set up as an integration directly between github and | ||
| 581 | +readthedocs. | ||
| 582 | + | ||
| 583 | +The way readthedocs.org does stable and versions doesn't exactly work | ||
| 584 | +for qpdf. My tagging convention is different from what they expect, | ||
| 585 | +and I don't need versions for every point release. I have the | ||
| 586 | +following branching strategy to support docs: | ||
| 587 | + | ||
| 588 | +* x.y -- points to the latest x.y.z release | ||
| 589 | +* stable -- points to the latest release | ||
| 590 | + | ||
| 591 | +The release process includes updating the approach branches and | ||
| 592 | +activating versions. | ||
| 593 | + | ||
| 594 | + | ||
| 595 | +## CMAKE notes | ||
| 596 | + | ||
| 597 | +To verify the various cmake options and their interactions, several | ||
| 598 | +manual tests were done: | ||
| 599 | + | ||
| 600 | +* Break installed qpdf executables (qpdf, fix-qdf, zlib-flate), the | ||
| 601 | + shared library, and DLL.h to ensure that other qpdf installations do | ||
| 602 | + not interfere with building from source | ||
| 603 | + | ||
| 604 | +* Build static only and shared only | ||
| 605 | + | ||
| 606 | +* Install separate components separately | ||
| 607 | + | ||
| 608 | +* Build only HTML docs and only PDF docs | ||
| 609 | + | ||
| 610 | +* Try MAINTAINER_MODE without BUILD_DOC | ||
| 611 | + | ||
| 612 | +We are using RelWithDebInfo for mingw and other non-Windows builds but | ||
| 613 | +Release for MSVC. There are linker warnings if MSVC is built with | ||
| 614 | +RelWithDebInfo when using external-libs. | ||
| 615 | + | ||
| 616 | + | ||
| 617 | +## ABI checks | ||
| 618 | + | ||
| 619 | +Note: the check_abi program requires [castxml](https://github.com/CastXML/CastXML) to be installed. | ||
| 620 | + | ||
| 621 | +Until the conversion of the build to cmake, we relied on running the | ||
| 622 | +test suite with old executables and a new library. When QPDFJob was | ||
| 623 | +introduced, this method got much less reliable since a lot of public | ||
| 624 | +API doesn't cross the shared library boundary. Also, when switching to | ||
| 625 | +cmake, we wanted a stronger check that the library had the expected | ||
| 626 | +ABI. | ||
| 627 | + | ||
| 628 | +Our ABI check now consists of three parts: | ||
| 629 | + | ||
| 630 | +* The same check as before: run the test suite with old executables | ||
| 631 | + and a new library | ||
| 632 | + | ||
| 633 | +* Do a literal comparison of the symbols in the old and new shared | ||
| 634 | + libraries -- this is a strong test of ABI change | ||
| 635 | + | ||
| 636 | +* Do a check to ensure that object sizes didn't change -- even with no | ||
| 637 | + changes to the API of exported functions, size changes break API | ||
| 638 | + | ||
| 639 | +The combination of these checks is pretty strong, though there are | ||
| 640 | +still things that could potentially break ABI, such as | ||
| 641 | + | ||
| 642 | +* Changes to the types of public or protected data members without | ||
| 643 | + changing the size | ||
| 644 | + | ||
| 645 | +* Changes to the meanings of parameters without changing the signature | ||
| 646 | + | ||
| 647 | +Not breaking ABI/API still requires care. | ||
| 648 | + | ||
| 649 | +The check_abi script is responsible for performing many of these | ||
| 650 | +steps. See comments in check_abi for additional notes. | ||
| 651 | + | ||
| 652 | + | ||
| 653 | +## CODE FORMATTING | ||
| 654 | + | ||
| 655 | +* Emacs doesn't indent breaking strings concatenated with + over | ||
| 656 | + lines but clang-format does. It's clearer with clang-format. To | ||
| 657 | + get emacs and clang-format to agree, parenthesize the expression | ||
| 658 | + that builds the concatenated string. | ||
| 659 | + | ||
| 660 | +* With | ||
| 661 | + | ||
| 662 | + ```cpp | ||
| 663 | + long_function(long_function( | ||
| 664 | + args) | ||
| 665 | + | ||
| 666 | + ``` | ||
| 667 | + | ||
| 668 | + clang-format anchors relative to the first function, and emacs | ||
| 669 | + anchors relative to the second function. Use | ||
| 670 | + | ||
| 671 | + ```cpp | ||
| 672 | + long_function( | ||
| 673 | + // line-break | ||
| 674 | + long_function( | ||
| 675 | + args) | ||
| 676 | + ``` | ||
| 677 | + to resolve. | ||
| 678 | + | ||
| 679 | +In the revision control history, there is a commit around April 3, | ||
| 680 | +2022 with the title "Update some code manually to get better | ||
| 681 | +formatting results" that shows several examples of changing code so | ||
| 682 | +that clang-format produces several results. (In git this is commit | ||
| 683 | +77e889495f7c513ba8677df5fe662f08053709eb.) | ||
| 684 | + | ||
| 685 | +The commits that have the bulk of automatic or mechanical reformatting are | ||
| 686 | +listed in .git-blame-ignore-revs. Any new bulk updates should be added there. | ||
| 687 | + | ||
| 688 | +[//]: # (cSpell:ignore pikepdfs readthedocsorg dgenerate .) |
README-maintainer.md
| 1 | # Maintainer Notes | 1 | # Maintainer Notes |
| 2 | 2 | ||
| 3 | +This file contains notes of interest only to maintainers of qpdf. | ||
| 4 | + | ||
| 5 | +For information also of interest to contributors and other developers that want to modify qpdf | ||
| 6 | +please see [README-developer.md](./README-developer.md). This information used to be part | ||
| 7 | +of this file, but was split off to make it easier for new developers to find relevant information. | ||
| 8 | + | ||
| 9 | + | ||
| 3 | ## Contents | 10 | ## Contents |
| 4 | 11 | ||
| 5 | -* [ROUTINE DEVELOPMENT](#routine-development) | ||
| 6 | * [VERSIONS](#versions) | 12 | * [VERSIONS](#versions) |
| 7 | -* [CHECKING DOCS ON readthedocs](#checking-docs-on-readthedocs) | ||
| 8 | * [GOOGLE OSS-FUZZ](#google-oss-fuzz) | 13 | * [GOOGLE OSS-FUZZ](#google-oss-fuzz) |
| 9 | -* [CODING RULES](#coding-rules) | ||
| 10 | -* [ZLIB COMPATIBILITY](#zlib-compatibility) | ||
| 11 | -* [HOW TO ADD A COMMAND-LINE ARGUMENT](#how-to-add-a-command-line-argument) | ||
| 12 | * [RELEASE PREPARATION](#release-preparation) | 14 | * [RELEASE PREPARATION](#release-preparation) |
| 13 | * [CREATING A RELEASE](#creating-a-release) | 15 | * [CREATING A RELEASE](#creating-a-release) |
| 14 | -* [RUNNING pikepdf's TEST SUITE](#running-pikepdfs-test-suite) | ||
| 15 | -* [OTHER NOTES](#other-notes) | ||
| 16 | -* [DEPRECATION](#deprecation) | ||
| 17 | -* [LOCAL WINDOWS TESTING PROCEDURE](#local-windows-testing-procedure) | ||
| 18 | -* [DOCS ON readthedocs.org](#docs-on-readthedocsorg) | ||
| 19 | -* [CMAKE notes](#cmake-notes) | ||
| 20 | -* [ABI checks](#abi-checks) | ||
| 21 | -* [CODE FORMATTING](#code-formatting) | ||
| 22 | - | ||
| 23 | - | ||
| 24 | -## ROUTINE DEVELOPMENT | ||
| 25 | - | ||
| 26 | -**When making changes that users need to know about, update the release notes | ||
| 27 | -(manual/release-notes.rst) as you go.** Major changes to the internal API can also be mentioned in | ||
| 28 | -the release notes in a section called "Internal Changes" or similar. This removes ChangeLog as a | ||
| 29 | -separate mechanism for tracking changes. | ||
| 30 | - | ||
| 31 | -**Remember to check pull requests as well as issues in github.** | ||
| 32 | - | ||
| 33 | -Run `cmake --list-presets` to see available cmake presets. Routine maintainer development can be | ||
| 34 | - | ||
| 35 | -``` | ||
| 36 | -cmake --preset maintainer | ||
| 37 | -cmake --build --preset maintainer | ||
| 38 | -ctest --preset maintainer | ||
| 39 | -``` | ||
| 40 | - | ||
| 41 | -See [CMakePresets.json](CMakePresets.json) for additional presets. Reminders about presets: | ||
| 42 | -* You can override/enhance configure presets, e.g., `cmake --preset maintainer -DCMAKE_BUILD_TYPE=Release` | ||
| 43 | -* You can pass flags to ctest, e.g., `ctest --preset maintainer -R zlib-flate`. | ||
| 44 | -* You can't override the build directory for build and test presets, but you _can_ override the | ||
| 45 | - directory for configure presets and then run `cmake --build build-dir` and `ctest` manually, as | ||
| 46 | - shown below. | ||
| 47 | -* The base configuration includes `-DCMAKE_EXPORT_COMPILE_COMMANDS=ON`, which is useful for LSP mode | ||
| 48 | - in C++. This is harmless in environments where it's not needed. You may need to make a symlink | ||
| 49 | - from compile_commands.json to the one in whichever build directory you are using. | ||
| 50 | -* If you have common configurations you'd like to see, pull requests are welcome, but | ||
| 51 | - `CMakeUserPresets.json` is your friend. You can copy or inherit from CMakeUserPresets.json for | ||
| 52 | - your own use. Note that CMakeUserPresets.json is not part of the stable API. We reserve the right | ||
| 53 | - to modify these presets in a non-compatible fashion at any time without regard to qpdf version | ||
| 54 | - numbers, but we should mention changes in the release notes. | ||
| 55 | -* Study the CMakePresets.json file for details on how these are implemented. | ||
| 56 | - | ||
| 57 | -See also ./build-scripts for other ways to run the build for different configurations. | ||
| 58 | - | ||
| 59 | -### Useful build examples | ||
| 60 | - | ||
| 61 | -To run a maintainer build in release mode and run only the unicode-filenames test, you could run | ||
| 62 | - | ||
| 63 | -``` | ||
| 64 | -cmake --preset maintainer -DCMAKE_BUILD_TYPE=Release | ||
| 65 | -cmake --build --preset maintainer | ||
| 66 | -TESTS=unicode-filenames ctest --preset maintainer -R qpdf | ||
| 67 | -``` | ||
| 68 | - | ||
| 69 | -To run a maintainer build in release mode in a _different directory_ and run only the | ||
| 70 | -unicode-filenames test, you could run the following. Trying to override the directory on the command | ||
| 71 | -line of `cmake --build` or `ctest` in conjunction with `--preset` may silently ignore the directory | ||
| 72 | -override, and you may not get what you think you are getting. | ||
| 73 | - | ||
| 74 | -``` | ||
| 75 | -cmake --preset maintainer -DCMAKE_BUILD_TYPE=Release -B cmake-build-release | ||
| 76 | -cmake --build cmake-build-release | ||
| 77 | -TESTS=unicode-filenames ctest --verbose --test-dir cmake-build-release -R qpdf | ||
| 78 | -``` | ||
| 79 | - | ||
| 80 | -### Profiling | ||
| 81 | - | ||
| 82 | -When running with the `maintainer-profile` preset (or any time you run profiling), run `gprof | ||
| 83 | -gmon.out`. Note that gmon.out is not cumulative. | ||
| 84 | - | ||
| 85 | -### Coverage | ||
| 86 | - | ||
| 87 | -When running with the `maintainer-coverage` preset, after running tests: | ||
| 88 | -``` | ||
| 89 | -gcovr -r .. --html --html-details -o coverage-report.html | ||
| 90 | -``` | ||
| 91 | 16 | ||
| 92 | -Note that, in early 2024, branch coverage information is not very accurate with C++. | ||
| 93 | - | ||
| 94 | -### Sanitizers/Memory Checks | ||
| 95 | - | ||
| 96 | -If `clang++` fails to create output during configuration, it may be necessary to install a specific | ||
| 97 | -version of libstdc++-dev. For example, with clang++ version 20 on Ubuntu 24.04, `clang++ -v` | ||
| 98 | -indicates the selected GCC installation is 14, so it is necessary to install `libstdc++-14-dev`. | ||
| 99 | - | ||
| 100 | -### Windows | ||
| 101 | - | ||
| 102 | -You can use this for command-line builds, which does a bit more than the presets. The msvc presets | ||
| 103 | -are known to work in CLion if the environment is set up as described in | ||
| 104 | -[README-windows.md](./README-windows.md), but for regular command-line builds (and CI), continue to | ||
| 105 | -use `cmake-win` from inside a build directory. Look at `build-scripts/build-windows` to see how this | ||
| 106 | -is used. | ||
| 107 | - | ||
| 108 | -``` | ||
| 109 | -../cmake-win {mingw|msvc} maint | ||
| 110 | -``` | ||
| 111 | 17 | ||
| 112 | ## VERSIONS | 18 | ## VERSIONS |
| 113 | 19 | ||
| @@ -133,11 +39,6 @@ is used. | @@ -133,11 +39,6 @@ is used. | ||
| 133 | there or the changes can be merged back, depending on the amount of | 39 | there or the changes can be merged back, depending on the amount of |
| 134 | drift. | 40 | drift. |
| 135 | 41 | ||
| 136 | -## CHECKING DOCS ON readthedocs | ||
| 137 | - | ||
| 138 | -To check docs on readthedocs.io without running all of CI, push to the | ||
| 139 | -doc-check branch. Then visit https://qpdf.readthedocs.io/en/doc-check/ | ||
| 140 | -Building docs from pull requests is also enabled. | ||
| 141 | 42 | ||
| 142 | ## GOOGLE OSS-FUZZ | 43 | ## GOOGLE OSS-FUZZ |
| 143 | 44 | ||
| @@ -188,319 +89,6 @@ Building docs from pull requests is also enabled. | @@ -188,319 +89,6 @@ Building docs from pull requests is also enabled. | ||
| 188 | * Latest corpus: | 89 | * Latest corpus: |
| 189 | gs://qpdf-backup.clusterfuzz-external.appspot.com/corpus/libFuzzer/qpdf_fuzzer/latest.zip | 90 | gs://qpdf-backup.clusterfuzz-external.appspot.com/corpus/libFuzzer/qpdf_fuzzer/latest.zip |
| 190 | 91 | ||
| 191 | -## CODING RULES | ||
| 192 | - | ||
| 193 | -* Code is formatted with clang-format. See .clang-format and the | ||
| 194 | - "Code Formatting" section in manual/contributing.rst for details. | ||
| 195 | - See also "CODE FORMATTING" below. | ||
| 196 | - | ||
| 197 | -* Use std::to_string instead of QUtil::int_to_string et al | ||
| 198 | - | ||
| 199 | -* Use of assert: | ||
| 200 | - | ||
| 201 | - * Test code: #include <qpdf/assert_test.h> first. | ||
| 202 | - * Debug code: #include <qpdf/assert_debug.h> first and use | ||
| 203 | - qpdf_assert_debug instead of assert. Note that <qpdf/Util.hh> | ||
| 204 | - includes assert_debug.h. Include this instead if 'At most one | ||
| 205 | - qpdf/assert header ...' errors are encountered, especially when | ||
| 206 | - using assert in private header files. | ||
| 207 | - * Use 'qpdf_expect', 'qpdf_static_expect', 'qpdf_ensures' and | ||
| 208 | - 'qpdf_invariant' to document pre/post-conditions and invariants. | ||
| 209 | - This requires inclusion of 'assert_debug.h' or 'Util.hh'. Remember | ||
| 210 | - that these (except for 'qpdf_static_expect') are only checked in | ||
| 211 | - debug builds. | ||
| 212 | - * Use 'util::assertion' when checks should also be carried out in | ||
| 213 | - release code in preference to throwing logic_errors directly | ||
| 214 | - unless it is practical and desirable to test violations during | ||
| 215 | - CI testing. This avoids obscuring genuine gaps in coverage with | ||
| 216 | - noise generated by unreachable sanity checks. | ||
| 217 | - | ||
| 218 | - These rules are enforced by the check-assert test. This practices | ||
| 219 | - serves to | ||
| 220 | - | ||
| 221 | - * remind us that assert in release code disappears and so should only | ||
| 222 | - be used to document pre/post conditions and invariants, and for | ||
| 223 | - sanity checks during development and testing; when doing so use | ||
| 224 | - a Debug build configuration. | ||
| 225 | - | ||
| 226 | - * protect us from using assert in test code without explicitly | ||
| 227 | - removing the NDEBUG definition, since that would cause the assert | ||
| 228 | - not to actually be testing anything in non-Debug build | ||
| 229 | - configurations. | ||
| 230 | - | ||
| 231 | - Prior to 12.3 assert_test.h and assert_debug.h shared the same header | ||
| 232 | - guard, which prevented the simultaneous inclusion of both headers. | ||
| 233 | - This was changed to permit the CI testing of private-API methods | ||
| 234 | - without loosing the use of assertions in private header files. | ||
| 235 | - | ||
| 236 | -* In a source file, include the header file that declares the source | ||
| 237 | - class first followed by a blank line. If a config file is needed | ||
| 238 | - first, put a blank line between that and the header followed by | ||
| 239 | - another blank line. This assures that each header file is included | ||
| 240 | - first at least once, thereby ensuring that it explicitly includes | ||
| 241 | - all the headers it needs, which in turn alleviates lots of header | ||
| 242 | - ordering problems. The blank line ensures that formatters don't | ||
| 243 | - mess this up by resorting the headers. | ||
| 244 | - | ||
| 245 | -* Avoid atoi. Use QUtil::string_to_int instead. It does | ||
| 246 | - overflow/underflow checking. | ||
| 247 | - | ||
| 248 | -* Avoid certain functions that tend to be macros or create compilation | ||
| 249 | - errors on some platforms. Known cases: strcasecmp, abs. Avoid min | ||
| 250 | - and max. If needed, std::min and std::max are okay to use in C++ | ||
| 251 | - code with <algorithm> included. | ||
| 252 | - | ||
| 253 | -* Remember to avoid using `operator[]` with `std::string` or | ||
| 254 | - `std::vector`. Instead, use `at()`. See README-hardening.md for | ||
| 255 | - details. | ||
| 256 | - | ||
| 257 | -* Use QIntC for type conversions -- see casting policy in docs. | ||
| 258 | - | ||
| 259 | -* Remember to imbue ostringstreams with std::locale::classic() before | ||
| 260 | - outputting numbers. This protects against the user's global locale | ||
| 261 | - altering otherwise deterministic values. (See github issue #459.) | ||
| 262 | - One could argue that error messages containing numbers should | ||
| 263 | - respect the user's locale, but I think it's more important for | ||
| 264 | - output to be consistent, since the messages in question are not | ||
| 265 | - really targeted at the end user. | ||
| 266 | - | ||
| 267 | -* Use QPDF_DLL on all methods that are to be exported in the shared | ||
| 268 | - library/DLL. Use QPDF_DLL_CLASS for all classes whose type | ||
| 269 | - information is needed. This is important for classes that are used | ||
| 270 | - as exceptions, subclassed, or tested with dynamic_cast across the | ||
| 271 | - the shared object boundary (or "shared library boundary" -- we may | ||
| 272 | - use either term in comments and documentation). In particular, | ||
| 273 | - anything new derived from Pipeline or InputSource should be marked | ||
| 274 | - with QPDF_DLL_CLASS. We shouldn't need to do it for QPDFObjectHelper | ||
| 275 | - or QPDFDocumentHelper subclasses since there's no reason to use | ||
| 276 | - dynamic_cast with those, but doing it anyway may help with some | ||
| 277 | - strange cases for mingw or with some code generators that may | ||
| 278 | - systematically do this for other reasons. | ||
| 279 | - | ||
| 280 | - IMPORTANT NOTE ABOUT QPDF_DLL_CLASS: On mingw, the vtable for a | ||
| 281 | - class with some virtual methods and no pure virtual methods seems | ||
| 282 | - often (always?) not to be generated if the destructor is inline or | ||
| 283 | - declared with `= default`. Therefore, for any class that is intended | ||
| 284 | - to be used as a base class and doesn't contain any pure virtual | ||
| 285 | - methods, you must declare the destructor in the header without | ||
| 286 | - `= default` and provide a non-inline implementation in the source | ||
| 287 | - file. Add this comment to the implementation: | ||
| 288 | - | ||
| 289 | - ```cpp | ||
| 290 | - // Must be explicit and not inline -- see QPDF_DLL_CLASS in | ||
| 291 | - // README-maintainer | ||
| 292 | - ``` | ||
| 293 | - | ||
| 294 | -* Put private member variables in std::unique_ptr<Members> for all | ||
| 295 | - public classes. Forward declare Members in the header file and define | ||
| 296 | - Members in the implementation file. One of the major benefits of | ||
| 297 | - defining Members in the implementation file is that it makes it easier | ||
| 298 | - to use private classes as data members and simplifies the include order. | ||
| 299 | - Remember that Members must be fully defined before the destructor of the | ||
| 300 | - main class. For an example of this pattern see class JSONHandler. | ||
| 301 | - | ||
| 302 | - Exception: indirection through std::unique_ptr<Members> incurs an overhead, | ||
| 303 | - so don't do it for: | ||
| 304 | - * (especially private) classes that are copied a lot, like QPDFObjectHandle | ||
| 305 | - and QPDFObject. | ||
| 306 | - * classes that are a shared pointer to another class, such as QPDFObjectHandle | ||
| 307 | - or JSON. | ||
| 308 | - | ||
| 309 | - For exported classes that do not use the member pattern for performance | ||
| 310 | - reasons it is worth considering adding a std::unique_ptr to an empty Members | ||
| 311 | - class initialized to nullptr to give the flexibility to add data members | ||
| 312 | - without breaking the ABI. | ||
| 313 | - | ||
| 314 | - Note that, as of qpdf 11, many public classes use `std::shared_ptr` | ||
| 315 | - instead. Changing this to `std::unique_ptr` is ABI-breaking. If the | ||
| 316 | - class doesn't allow copying, we can switch it to std::unique_ptr and | ||
| 317 | - let that be the thing that prevents copying. If the intention is to | ||
| 318 | - allow the object to be copied by value and treated as if it were | ||
| 319 | - copied by reference, then `std::shared_ptr<Members>` should be used. | ||
| 320 | - The `JSON` class is an example of this. As a rule, we should avoid | ||
| 321 | - this design pattern. It's better to make things non-copiable and to | ||
| 322 | - require explicit use of shared pointers, so going forward, | ||
| 323 | - `std::unique_ptr` should be preferred. | ||
| 324 | - | ||
| 325 | -* Traversal of objects is expensive. It's worth adding some complexity | ||
| 326 | - to avoid needless traversals of objects. | ||
| 327 | - | ||
| 328 | -* Avoid attaching too much metadata to objects and object handles | ||
| 329 | - since those have to get copied around a lot. | ||
| 330 | - | ||
| 331 | -* Prefer std::string_view to std::string const& and char const*. | ||
| 332 | - | ||
| 333 | - * Where functions rely on strings being null-terminated, std::string_view may not be appropriate. | ||
| 334 | - | ||
| 335 | - * For return values, consider whether returning a string_view is safe or whether it is more appropriate | ||
| 336 | - to return a std::string or std::string const&, especially in the public API. | ||
| 337 | - | ||
| 338 | - * NEVER replace a std::string const& return value with std::string_view in the public API. | ||
| 339 | - | ||
| 340 | - | ||
| 341 | -## ZLIB COMPATIBILITY | ||
| 342 | - | ||
| 343 | -The qpdf test suite is designed to be independent of the output of any | ||
| 344 | -particular version of zlib. (See also `ZOPFLI` in README.md.) There | ||
| 345 | -are several strategies to make this work: | ||
| 346 | - | ||
| 347 | -* `build-scripts/test-alt-zlib` runs in CI and runs the test suite | ||
| 348 | - with a non-default zlib. Please refer to that code for an example of | ||
| 349 | - how to do this in case you want to test locally. | ||
| 350 | - | ||
| 351 | -* The test suite is full of cases that compare output PDF files with | ||
| 352 | - expected PDF files in the test suite. If the file contains data that | ||
| 353 | - was compressed by QPDFWriter, then the output file will depend on | ||
| 354 | - the behavior of zlib. As such, using a simple comparison won't work. | ||
| 355 | - There are several strategies used by the test suite. | ||
| 356 | - | ||
| 357 | - * A new program called `qpdf-test-compare`, in most cases, is a drop | ||
| 358 | - in replacement for a simple file comparison. This code make sure | ||
| 359 | - the two files have exactly the same number of objects with the | ||
| 360 | - same object and generation numbers, and that corresponding objects | ||
| 361 | - are identical with the following allowances (consult its source | ||
| 362 | - code for all the details details): | ||
| 363 | - * The `/Length` key is not compared in stream dictionaries. | ||
| 364 | - * The second element of `/ID` is not compared. | ||
| 365 | - * If the first and second element of `/ID` are the same, then the | ||
| 366 | - first element if `/ID` is also not compared. | ||
| 367 | - * If a stream is compressed with `/FlateDecode`, the | ||
| 368 | - _uncompressed_ stream data is compared. Otherwise, the raw | ||
| 369 | - stream data is compared. | ||
| 370 | - * Generated fields in the `/Encrypt` dictionary are not compared, | ||
| 371 | - though password-protected files must have the same password. | ||
| 372 | - * Differences in the contents of `/XRef` streams are ignored. | ||
| 373 | - | ||
| 374 | - To use this, run `qpdf-test-compare actual.pdf expected.pdf`, and | ||
| 375 | - expect the output to match `expected.pdf`. For example, if a test | ||
| 376 | - used to be written like this; | ||
| 377 | - ```perl | ||
| 378 | - $td->runtest("check output", | ||
| 379 | - {$td->FILE => "a.pdf"}, | ||
| 380 | - {$td->FILE => "out.pdf"}); | ||
| 381 | - ``` | ||
| 382 | - then write it like this instead: | ||
| 383 | - ```perl | ||
| 384 | - $td->runtest("check output", | ||
| 385 | - {$td->COMMAND => "qpdf-test-compare a.pdf out.pdf"}, | ||
| 386 | - {$td->FILE => "out.pdf", $td->EXIT_STATUS => 0}); | ||
| 387 | - ``` | ||
| 388 | - You can look at `compare-for-test/qtest/compare.test` for | ||
| 389 | - additional examples. | ||
| 390 | - | ||
| 391 | - Here's what's going on: | ||
| 392 | - * If the files "match" according to the rules of | ||
| 393 | - `qpdf-test-compare`, the output of the program is the expected | ||
| 394 | - file. | ||
| 395 | - * If the files do not match, the output is the actual file. The | ||
| 396 | - reason is that, if a change is made that results in an expected | ||
| 397 | - change to the expected file, the output of the comparison can be | ||
| 398 | - used to replace the expected file (as long as it is definitely | ||
| 399 | - known to be correct—no shortcuts here!). That way, it doesn't | ||
| 400 | - matter which zlib you use to generate test files. | ||
| 401 | - * As a special debugging tool, you can set the `QPDF_COMPARE_WHY` | ||
| 402 | - environment variable to any value. In this case, if the files | ||
| 403 | - don't match, the output is a description of the first thing in | ||
| 404 | - the file that doesn't match. This is mostly useful for debugging | ||
| 405 | - `qpdf-test-compare` itself, but it can also be helpful as a | ||
| 406 | - sanity check that the differences are expected. If you are | ||
| 407 | - trying to find out the _real_ differences, a suggestion is to | ||
| 408 | - convert both files to qdf and compare them lexically. | ||
| 409 | - | ||
| 410 | - * There are some cases where `qpdf-test-compare` can't be used. For | ||
| 411 | - example, if you need to actually test one of the things that | ||
| 412 | - `qpdf-test-compare` ignores, you'll need some other mechanism. | ||
| 413 | - There are tests for deterministic ID creation and xref streams | ||
| 414 | - that have to implement other mechanisms. Also, linearization hint | ||
| 415 | - streams and the linearization dictionary in a linearized file | ||
| 416 | - contain file offsets. Rather than ignoring those, it can be | ||
| 417 | - helpful to create linearized files using `--compress-streams=n`. | ||
| 418 | - In that case, `QPDFWriter` won't compress any data, so the PDF | ||
| 419 | - will be independent of the output of any particular zlib | ||
| 420 | - implementation. | ||
| 421 | - | ||
| 422 | -You can find many examples of how tests were rewritten by looking at | ||
| 423 | -the commits preceding the one that added this section of this README | ||
| 424 | -file. | ||
| 425 | - | ||
| 426 | -Note about `/ID`: many test cases use `--static-id` to have a | ||
| 427 | -predictable `/ID` for testing. Many other test cases use | ||
| 428 | -`--deterministic-id`. While `--static-id` is unaffected by file | ||
| 429 | -contents, `--deterministic-id` is based on file contents and so is | ||
| 430 | -dependent on zlib output if there is any newly compressed data. By | ||
| 431 | -using `qpdf-test-compare`, it's actually not necessary to use either | ||
| 432 | -`--static-id` or `--deterministic-id`. It may still be necessary to | ||
| 433 | -use `--static-aes-iv` if comparing encrypted files, but since | ||
| 434 | -`qpdf-test-compare` ignores `/Perms`, a wider range of encrypted files | ||
| 435 | -can be compared using `qpdf-test-compare`. | ||
| 436 | - | ||
| 437 | -## HOW TO ADD A COMMAND-LINE ARGUMENT | ||
| 438 | - | ||
| 439 | -Quick reminder: | ||
| 440 | - | ||
| 441 | -* Add an entry to the top half of job.yml for the command-line | ||
| 442 | - argument | ||
| 443 | -* Add an entry to the bottom half of job.yml for the job JSON field | ||
| 444 | -* Add documentation for the new option to cli.rst | ||
| 445 | -* Implement the QPDFJob::Config method in QPDFJob_config.cc | ||
| 446 | -* Pass the build option `-DGENERATE_AUTO_JOB=1` to `cmake` | ||
| 447 | - (see [here](https://qpdf.readthedocs.io/en/stable/installation.html#options-for-working-on-qpdf)) | ||
| 448 | - or run `generate_auto_job` manually. | ||
| 449 | -* Adding new options tables is harder -- see below | ||
| 450 | - | ||
| 451 | -QPDFJob is documented in three places: | ||
| 452 | - | ||
| 453 | -* This section provides a quick reminder for how to add a command-line | ||
| 454 | - argument | ||
| 455 | - | ||
| 456 | -* generate_auto_job has a detailed explanation about how QPDFJob and | ||
| 457 | - generate_auto_job work together | ||
| 458 | - | ||
| 459 | -* The manual ("QPDFJob Design" in qpdf-job.rst) discusses the design | ||
| 460 | - approach, rationale, and evolution of QPDFJob. | ||
| 461 | - | ||
| 462 | -Command-line arguments are closely coupled with QPDFJob. To add a new | ||
| 463 | -command-line argument, add the option to the appropriate table in | ||
| 464 | -job.yml. After `generate_auto_job` is run (either manually or as part | ||
| 465 | -of the build process, when `GENERATE_AUTO_JOB` is set), this will | ||
| 466 | -automatically declare a method in the private ArgParser class in | ||
| 467 | -QPDFJob_argv.cc which you have to implement. The implementation should | ||
| 468 | -make calls to methods in QPDFJob via its Config classes. Then, add the | ||
| 469 | -same option to either the no-json section of job.yml if it is to be | ||
| 470 | -excluded from the job json structure, or add it under the json | ||
| 471 | -structure to the place where it should appear in the json structure. | ||
| 472 | - | ||
| 473 | -In most cases, adding a new option will automatically declare and call | ||
| 474 | -the appropriate Config method, which you then have to implement. If | ||
| 475 | -you need a manual handler, you have to declare the option as manual in | ||
| 476 | -job.yml and implement the handler yourself, though the automatically | ||
| 477 | -generated code will declare it for you. | ||
| 478 | - | ||
| 479 | -Adding a new option table is a bit harder and is not well-documented. | ||
| 480 | -For a simple example, look at the code that added the | ||
| 481 | ---set-page-labels table. That change was divided into two commits (one | ||
| 482 | -for the manual changes, and one for the generated changes) to make it | ||
| 483 | -easier to use as an example. | ||
| 484 | - | ||
| 485 | -The build will fail until the new option is documented in | ||
| 486 | -manual/cli.rst. To do that, create documentation for the option by | ||
| 487 | -adding a ".. qpdf:option::" directive followed by a magic help comment | ||
| 488 | -as described at the top of manual/cli.rst. Put this in the correct | ||
| 489 | -help topic. Help topics roughly correspond with sections in that | ||
| 490 | -chapter and are created using a special ".. help-topic" comment. | ||
| 491 | -Follow the example of other options for style. | ||
| 492 | - | ||
| 493 | -When done, the following should happen: | ||
| 494 | - | ||
| 495 | -* qpdf --new-option should work as expected | ||
| 496 | -* qpdf --help=--new-option should show the help from the comment in cli.rst | ||
| 497 | -* qpdf --help=topic should list --new-option for the correct topic | ||
| 498 | -* --new-option should appear in the manual | ||
| 499 | -* --new-option should be in the command-line option index in the manual | ||
| 500 | -* A Config method (in Config or one of the other Config classes in | ||
| 501 | - QPDFJob) should exist that corresponds to the command-line flag | ||
| 502 | -* The job JSON file should have a new key in the schema corresponding | ||
| 503 | - to the new option | ||
| 504 | 92 | ||
| 505 | ## RELEASE PREPARATION | 93 | ## RELEASE PREPARATION |
| 506 | 94 | ||
| @@ -706,254 +294,4 @@ rsync -n -vrlcO $release/ sourceforge_login,qpdf@frs.sourceforge.net:/home/frs/p | @@ -706,254 +294,4 @@ rsync -n -vrlcO $release/ sourceforge_login,qpdf@frs.sourceforge.net:/home/frs/p | ||
| 706 | 294 | ||
| 707 | * Email the qpdf-announce list. Mention the email address of the release signer. | 295 | * Email the qpdf-announce list. Mention the email address of the release signer. |
| 708 | 296 | ||
| 709 | -## RUNNING pikepdf's TEST SUITE | ||
| 710 | - | ||
| 711 | -We run pikepdf's test suite from CI. These instructions show how to do | ||
| 712 | -it manually. | ||
| 713 | - | ||
| 714 | -Do this in a separate shell. | ||
| 715 | - | ||
| 716 | -``` | ||
| 717 | -cd ...qpdf-source-tree... | ||
| 718 | -export QPDF_SOURCE_TREE=$PWD | ||
| 719 | -export QPDF_BUILD_LIBDIR=$QPDF_SOURCE_TREE/build/libqpdf | ||
| 720 | -export LD_LIBRARY_PATH=$QPDF_BUILD_LIBDIR | ||
| 721 | -rm -rf /tmp/z | ||
| 722 | -mkdir /tmp/z | ||
| 723 | -cd /tmp/z | ||
| 724 | -git clone git@github.com:pikepdf/pikepdf | ||
| 725 | -python3 -m venv v | ||
| 726 | -source v/bin/activate | ||
| 727 | -cd pikepdf | ||
| 728 | -python3 -m pip install --upgrade pip | ||
| 729 | -python3 -m pip install '.[test]' | ||
| 730 | -rehash | ||
| 731 | -python3 -m pip install . | ||
| 732 | -pytest -n auto | ||
| 733 | -``` | ||
| 734 | - | ||
| 735 | -If there are failures, use git bisect to figure out where the failure | ||
| 736 | -was introduced. For example, set up a work area like this: | ||
| 737 | - | ||
| 738 | -``` | ||
| 739 | -rm -rf /tmp/z | ||
| 740 | -mkdir /tmp/z | ||
| 741 | -cd /tmp/z | ||
| 742 | -git clone file://$HOME/source/qpdf/qpdf/.git qpdf | ||
| 743 | -git clone git@github.com:pikepdf/pikepdf | ||
| 744 | -export QPDF_SOURCE_TREE=/tmp/z/qpdf | ||
| 745 | -export QPDF_BUILD_LIBDIR=$QPDF_SOURCE_TREE/build/libqpdf | ||
| 746 | -export LD_LIBRARY_PATH=$QPDF_BUILD_LIBDIR | ||
| 747 | -cd qpdf | ||
| 748 | -mkdir build | ||
| 749 | -cmake -B build -DMAINTAINER_MODE=ON -DBUILD_STATIC_LIBS=OFF \ | ||
| 750 | - -DCMAKE_BUILD_TYPE=RelWithDebInfo | ||
| 751 | -cat <<'EOF' | ||
| 752 | -#!/bin/bash | ||
| 753 | -cd /tmp/z/pikepdf | ||
| 754 | -cmake --build /tmp/z/qpdf/build -j16 --target libqpdf -- -k | ||
| 755 | -git clean -dfx | ||
| 756 | -rm -rf ../v | ||
| 757 | -python3 -m venv ../v | ||
| 758 | -source ../v/bin/activate | ||
| 759 | -python3 -m pip install --upgrade pip | ||
| 760 | -python3 -m pip install '.[test]' | ||
| 761 | -python3 -m pip install . | ||
| 762 | -pytest -n auto | ||
| 763 | -EOF | ||
| 764 | -chmod +x /tmp/check | ||
| 765 | -``` | ||
| 766 | - | ||
| 767 | -Then in /tmp/z/qpdf, run git bisect. Use /tmp/check at each stage to | ||
| 768 | -test whether it's a good or bad commit. | ||
| 769 | - | ||
| 770 | -## OTHER NOTES | ||
| 771 | - | ||
| 772 | -For local iteration on the AppImage generation, it works to just | ||
| 773 | -./build-scripts/build-appimage and get the resulting AppImage from the | ||
| 774 | -distribution directory. You can pass additional arguments to | ||
| 775 | -build-appimage, which passes them along to to docker. | ||
| 776 | - | ||
| 777 | -Use -e SKIP_TESTS=1 to skip the test suite. | ||
| 778 | -Use -ti -e RUN_SHELL=1 to run a shell instead of the build script. | ||
| 779 | - | ||
| 780 | -To iterate on the scripts directly in the source tree, you can run | ||
| 781 | - | ||
| 782 | -``` | ||
| 783 | -docker build -t qpdfbuild appimage | ||
| 784 | -docker run --privileged --rm -ti -e SKIP_TESTS=1 -e RUN_SHELL=1 \ | ||
| 785 | - -v $PWD/..:/tmp/build ${1+"$@"} qpdfbuild | ||
| 786 | -``` | ||
| 787 | - | ||
| 788 | -This will put you at a shell prompt inside the container with your | ||
| 789 | -current directory set to the top of the source tree and your uid equal | ||
| 790 | -to the owner of the parent directory source tree. | ||
| 791 | - | ||
| 792 | -Note: this will leave some extra files (like .bash_history) in the | ||
| 793 | -parent directory of the source tree. You will want to clean those up. | ||
| 794 | - | ||
| 795 | -## DEPRECATION | ||
| 796 | - | ||
| 797 | -This is a reminder of how to use and test deprecation. | ||
| 798 | - | ||
| 799 | -To temporarily disable deprecation warnings for testing: | ||
| 800 | - | ||
| 801 | -```cpp | ||
| 802 | -#ifdef _MSC_VER | ||
| 803 | -# pragma warning(disable : 4996) | ||
| 804 | -#endif | ||
| 805 | -#if (defined(__GNUC__) || defined(__clang__)) | ||
| 806 | -# pragma GCC diagnostic push | ||
| 807 | -# pragma GCC diagnostic ignored "-Wdeprecated-declarations" | ||
| 808 | -#endif | ||
| 809 | - // Do deprecated thing here | ||
| 810 | -#if (defined(__GNUC__) || defined(__clang__)) | ||
| 811 | -# pragma GCC diagnostic pop | ||
| 812 | -#endif | ||
| 813 | -``` | ||
| 814 | - | ||
| 815 | -To declare something as deprecated: | ||
| 816 | - | ||
| 817 | -```cpp | ||
| 818 | -[[deprecated("explanation")]] | ||
| 819 | -``` | ||
| 820 | - | ||
| 821 | -## LOCAL WINDOWS TESTING PROCEDURE | ||
| 822 | - | ||
| 823 | -This is what I do for routine testing on Windows. | ||
| 824 | - | ||
| 825 | -* From Windows, git clone from my Linux clone, and unzip | ||
| 826 | - `external-libs`. | ||
| 827 | - | ||
| 828 | -* Start a command-line shell for x86_64 and x86 tools from Visual | ||
| 829 | - studio. From there, start C:\msys64\mingw64 twice and | ||
| 830 | - C:\msys64\mingw32 twice. | ||
| 831 | - | ||
| 832 | -* Create a build directory for each of the four permutations. Then, in | ||
| 833 | - each build directory, run `../cmake-win <tool> maint`. | ||
| 834 | - | ||
| 835 | -* Run `cmake --build . -j4`. For MSVC, add `--config Release` | ||
| 836 | - | ||
| 837 | -* Test with with msvc: `ctest --verbose -C Release` | ||
| 838 | - | ||
| 839 | -* Test with mingw: `ctest --verbose -C RelWithDebInfo` | ||
| 840 | - | ||
| 841 | -## DOCS ON readthedocs.org | ||
| 842 | - | ||
| 843 | -* Registered for an account at readthedocs.org with my github account | ||
| 844 | -* Project page: https://readthedocs.org/projects/qpdf/ | ||
| 845 | -* Docs: https://qpdf.readthedocs.io/ | ||
| 846 | -* Admin -> Settings | ||
| 847 | - * Set project home page | ||
| 848 | - * Advanced | ||
| 849 | - * Show version warning | ||
| 850 | - * Default version: stable | ||
| 851 | - * Email Notifications: set email address for build failures | ||
| 852 | - | ||
| 853 | -At this time, there is nothing in .github/workflows to support this. | ||
| 854 | -It's all set up as an integration directly between github and | ||
| 855 | -readthedocs. | ||
| 856 | - | ||
| 857 | -The way readthedocs.org does stable and versions doesn't exactly work | ||
| 858 | -for qpdf. My tagging convention is different from what they expect, | ||
| 859 | -and I don't need versions for every point release. I have the | ||
| 860 | -following branching strategy to support docs: | ||
| 861 | - | ||
| 862 | -* x.y -- points to the latest x.y.z release | ||
| 863 | -* stable -- points to the latest release | ||
| 864 | - | ||
| 865 | -The release process includes updating the approach branches and | ||
| 866 | -activating versions. | ||
| 867 | - | ||
| 868 | -## CMAKE notes | ||
| 869 | - | ||
| 870 | -To verify the various cmake options and their interactions, several | ||
| 871 | -manual tests were done: | ||
| 872 | - | ||
| 873 | -* Break installed qpdf executables (qpdf, fix-qdf, zlib-flate), the | ||
| 874 | - shared library, and DLL.h to ensure that other qpdf installations do | ||
| 875 | - not interfere with building from source | ||
| 876 | - | ||
| 877 | -* Build static only and shared only | ||
| 878 | - | ||
| 879 | -* Install separate components separately | ||
| 880 | - | ||
| 881 | -* Build only HTML docs and only PDF docs | ||
| 882 | - | ||
| 883 | -* Try MAINTAINER_MODE without BUILD_DOC | ||
| 884 | - | ||
| 885 | -We are using RelWithDebInfo for mingw and other non-Windows builds but | ||
| 886 | -Release for MSVC. There are linker warnings if MSVC is built with | ||
| 887 | -RelWithDebInfo when using external-libs. | ||
| 888 | - | ||
| 889 | -## ABI checks | ||
| 890 | - | ||
| 891 | -Note: the check_abi program requires [castxml](https://github.com/CastXML/CastXML) to be installed. | ||
| 892 | - | ||
| 893 | -Until the conversion of the build to cmake, we relied on running the | ||
| 894 | -test suite with old executables and a new library. When QPDFJob was | ||
| 895 | -introduced, this method got much less reliable since a lot of public | ||
| 896 | -API doesn't cross the shared library boundary. Also, when switching to | ||
| 897 | -cmake, we wanted a stronger check that the library had the expected | ||
| 898 | -ABI. | ||
| 899 | - | ||
| 900 | -Our ABI check now consists of three parts: | ||
| 901 | - | ||
| 902 | -* The same check as before: run the test suite with old executables | ||
| 903 | - and a new library | ||
| 904 | - | ||
| 905 | -* Do a literal comparison of the symbols in the old and new shared | ||
| 906 | - libraries -- this is a strong test of ABI change | ||
| 907 | - | ||
| 908 | -* Do a check to ensure that object sizes didn't change -- even with no | ||
| 909 | - changes to the API of exported functions, size changes break API | ||
| 910 | - | ||
| 911 | -The combination of these checks is pretty strong, though there are | ||
| 912 | -still things that could potentially break ABI, such as | ||
| 913 | - | ||
| 914 | -* Changes to the types of public or protected data members without | ||
| 915 | - changing the size | ||
| 916 | - | ||
| 917 | -* Changes to the meanings of parameters without changing the signature | ||
| 918 | - | ||
| 919 | -Not breaking ABI/API still requires care. | ||
| 920 | - | ||
| 921 | -The check_abi script is responsible for performing many of these | ||
| 922 | -steps. See comments in check_abi for additional notes. | ||
| 923 | - | ||
| 924 | -## CODE FORMATTING | ||
| 925 | - | ||
| 926 | -* Emacs doesn't indent breaking strings concatenated with + over | ||
| 927 | - lines but clang-format does. It's clearer with clang-format. To | ||
| 928 | - get emacs and clang-format to agree, parenthesize the expression | ||
| 929 | - that builds the concatenated string. | ||
| 930 | - | ||
| 931 | -* With | ||
| 932 | - | ||
| 933 | - ```cpp | ||
| 934 | - long_function(long_function( | ||
| 935 | - args) | ||
| 936 | - | ||
| 937 | - ``` | ||
| 938 | - | ||
| 939 | - clang-format anchors relative to the first function, and emacs | ||
| 940 | - anchors relative to the second function. Use | ||
| 941 | - | ||
| 942 | - ```cpp | ||
| 943 | - long_function( | ||
| 944 | - // line-break | ||
| 945 | - long_function( | ||
| 946 | - args) | ||
| 947 | - ``` | ||
| 948 | - to resolve. | ||
| 949 | - | ||
| 950 | -In the revision control history, there is a commit around April 3, | ||
| 951 | -2022 with the title "Update some code manually to get better | ||
| 952 | -formatting results" that shows several examples of changing code so | ||
| 953 | -that clang-format produces several results. (In git this is commit | ||
| 954 | -77e889495f7c513ba8677df5fe662f08053709eb.) | ||
| 955 | - | ||
| 956 | -The commits that have the bulk of automatic or mechanical reformatting are | ||
| 957 | -listed in .git-blame-ignore-revs. Any new bulk updates should be added there. | ||
| 958 | - | ||
| 959 | [//]: # (cSpell:ignore pikepdfs readthedocsorg dgenerate .) | 297 | [//]: # (cSpell:ignore pikepdfs readthedocsorg dgenerate .) |
build-scripts/test-alt-zlib
| @@ -35,5 +35,5 @@ if [ "$sum1" = "$sum2" ]; then | @@ -35,5 +35,5 @@ if [ "$sum1" = "$sum2" ]; then | ||
| 35 | exit 2 | 35 | exit 2 |
| 36 | fi | 36 | fi |
| 37 | 37 | ||
| 38 | -# If this fails, please see ZLIB COMPATIBILITY in README-maintainer.md. | 38 | +# If this fails, please see ZLIB COMPATIBILITY in README-developer.md. |
| 39 | (cd build; ctest --verbose) | 39 | (cd build; ctest --verbose) |
build-scripts/test-zopfli
| @@ -20,7 +20,7 @@ if [ "$zopfli" != "11" ]; then | @@ -20,7 +20,7 @@ if [ "$zopfli" != "11" ]; then | ||
| 20 | exit 2 | 20 | exit 2 |
| 21 | fi | 21 | fi |
| 22 | 22 | ||
| 23 | -# If this fails, please see ZLIB COMPATIBILITY in README-maintainer.md. | 23 | +# If this fails, please see ZLIB COMPATIBILITY in README-developer.md. |
| 24 | # The tests are very slow with this option. Just run essential tests. | 24 | # The tests are very slow with this option. Just run essential tests. |
| 25 | # If zlib-flate and qpdf tests all pass, we can be pretty sure it works. | 25 | # If zlib-flate and qpdf tests all pass, we can be pretty sure it works. |
| 26 | (cd build; ctest --verbose -R zlib-flate) | 26 | (cd build; ctest --verbose -R zlib-flate) |
examples/qpdfjob-remove-annotations.cc
| @@ -35,7 +35,7 @@ realmain(int argc, char* argv[]) | @@ -35,7 +35,7 @@ realmain(int argc, char* argv[]) | ||
| 35 | 35 | ||
| 36 | QPDFJob j; | 36 | QPDFJob j; |
| 37 | try { | 37 | try { |
| 38 | - // See "HOW TO ADD A COMMAND-LINE ARGUMENT" in README-maintainer. | 38 | + // See "HOW TO ADD A COMMAND-LINE ARGUMENT" in README-developer. |
| 39 | j.initializeFromArgv(argv); | 39 | j.initializeFromArgv(argv); |
| 40 | auto qpdf_sp = j.createQPDF(); | 40 | auto qpdf_sp = j.createQPDF(); |
| 41 | auto& pdf = *qpdf_sp; | 41 | auto& pdf = *qpdf_sp; |
generate_auto_job
| @@ -16,7 +16,7 @@ from contextlib import contextmanager | @@ -16,7 +16,7 @@ from contextlib import contextmanager | ||
| 16 | 16 | ||
| 17 | # Documentation of QPDFJob is divided among three places: | 17 | # Documentation of QPDFJob is divided among three places: |
| 18 | # | 18 | # |
| 19 | -# * "HOW TO ADD A COMMAND-LINE ARGUMENT" in README-maintainer provides | 19 | +# * "HOW TO ADD A COMMAND-LINE ARGUMENT" in README-developer provides |
| 20 | # a quick reminder for how to add a command-line argument | 20 | # a quick reminder for how to add a command-line argument |
| 21 | # | 21 | # |
| 22 | # * This file has a detailed explanation about how QPDFJob and | 22 | # * This file has a detailed explanation about how QPDFJob and |
include/qpdf/QPDFJob.hh
| @@ -180,7 +180,7 @@ class QPDFJob | @@ -180,7 +180,7 @@ class QPDFJob | ||
| 180 | // needed, which you can't do with references. Returning pointers instead of references makes | 180 | // needed, which you can't do with references. Returning pointers instead of references makes |
| 181 | // for a more uniform interface. | 181 | // for a more uniform interface. |
| 182 | 182 | ||
| 183 | - // Maintainer documentation: see the section in README-maintainer called "HOW TO ADD A | 183 | + // Maintainer documentation: see the section in README-developer called "HOW TO ADD A |
| 184 | // COMMAND-LINE ARGUMENT", which contains references to additional places in the documentation. | 184 | // COMMAND-LINE ARGUMENT", which contains references to additional places in the documentation. |
| 185 | 185 | ||
| 186 | class Config; | 186 | class Config; |
job.sums
| 1 | # Generated by generate_auto_job | 1 | # Generated by generate_auto_job |
| 2 | CMakeLists.txt 9ed8b0574b60d0625004b4f578089166d10c3d2f2bfc61edf94c105020d82264 | 2 | CMakeLists.txt 9ed8b0574b60d0625004b4f578089166d10c3d2f2bfc61edf94c105020d82264 |
| 3 | -generate_auto_job 8e3175a515aa8837d8a01bba0346b04b3d777d70330ba5b7d52f691316054a34 | 3 | +generate_auto_job 71df64ab48364b1f77a8fafa0ba075bb4bad2a65f2a6a6322d51a1a888f83198 |
| 4 | include/qpdf/auto_job_c_att.hh 4c2b171ea00531db54720bf49a43f8b34481586ae7fb6cbf225099ee42bc5bb4 | 4 | include/qpdf/auto_job_c_att.hh 4c2b171ea00531db54720bf49a43f8b34481586ae7fb6cbf225099ee42bc5bb4 |
| 5 | include/qpdf/auto_job_c_copy_att.hh 50609012bff14fd82f0649185940d617d05d530cdc522185c7f3920a561ccb42 | 5 | include/qpdf/auto_job_c_copy_att.hh 50609012bff14fd82f0649185940d617d05d530cdc522185c7f3920a561ccb42 |
| 6 | include/qpdf/auto_job_c_enc.hh 28446f3c32153a52afa239ea40503e6cc8ac2c026813526a349e0cd4ae17ddd5 | 6 | include/qpdf/auto_job_c_enc.hh 28446f3c32153a52afa239ea40503e6cc8ac2c026813526a349e0cd4ae17ddd5 |
| @@ -8,7 +8,7 @@ include/qpdf/auto_job_c_global.hh 7df0ff87d18d7fa6d57437960377509420b6b6eb9527b5 | @@ -8,7 +8,7 @@ include/qpdf/auto_job_c_global.hh 7df0ff87d18d7fa6d57437960377509420b6b6eb9527b5 | ||
| 8 | include/qpdf/auto_job_c_main.hh 7a7cc311e194b778ea7568ece2335c00c9bdd8162431c264af2f6105548efc5f | 8 | include/qpdf/auto_job_c_main.hh 7a7cc311e194b778ea7568ece2335c00c9bdd8162431c264af2f6105548efc5f |
| 9 | include/qpdf/auto_job_c_pages.hh 09ca15649cc94fdaf6d9bdae28a20723f2a66616bf15aa86d83df31051d82506 | 9 | include/qpdf/auto_job_c_pages.hh 09ca15649cc94fdaf6d9bdae28a20723f2a66616bf15aa86d83df31051d82506 |
| 10 | include/qpdf/auto_job_c_uo.hh 9c2f98a355858dd54d0bba444b73177a59c9e56833e02fa6406f429c07f39e62 | 10 | include/qpdf/auto_job_c_uo.hh 9c2f98a355858dd54d0bba444b73177a59c9e56833e02fa6406f429c07f39e62 |
| 11 | -job.yml 442ef5e7fb8027a0f3cd4816b614d59abb327b359e861d87426b706db142d2bc | 11 | +job.yml 0306e1502689ad4e6cca78b0074088e0ab759f2e9f38e63822c8a824dcfaddfa |
| 12 | libqpdf/qpdf/auto_job_decl.hh d612a02839e4f20a80e1c6a3ba09c17187fccddc3581ec7ebb1e3919ffd6801d | 12 | libqpdf/qpdf/auto_job_decl.hh d612a02839e4f20a80e1c6a3ba09c17187fccddc3581ec7ebb1e3919ffd6801d |
| 13 | libqpdf/qpdf/auto_job_help.hh 43d0c1f4477949d1a4facc6984e9e341758201f4884e8d0d82ffc8dd795b2d38 | 13 | libqpdf/qpdf/auto_job_help.hh 43d0c1f4477949d1a4facc6984e9e341758201f4884e8d0d82ffc8dd795b2d38 |
| 14 | libqpdf/qpdf/auto_job_init.hh 2bb9d1524817e7150da55790ba0ac5ed8042ff4ff9688d70a927ee95903849a4 | 14 | libqpdf/qpdf/auto_job_init.hh 2bb9d1524817e7150da55790ba0ac5ed8042ff4ff9688d70a927ee95903849a4 |
| @@ -16,6 +16,6 @@ libqpdf/qpdf/auto_job_json_decl.hh 7dbb83ddadcea39bfd1faa4ca061e1e3c3134d693b8ae | @@ -16,6 +16,6 @@ libqpdf/qpdf/auto_job_json_decl.hh 7dbb83ddadcea39bfd1faa4ca061e1e3c3134d693b8ae | ||
| 16 | libqpdf/qpdf/auto_job_json_init.hh 33934c23235c760a1fc1375c74f4fd52c8f5accf655c510fa3dd9a2be293b61d | 16 | libqpdf/qpdf/auto_job_json_init.hh 33934c23235c760a1fc1375c74f4fd52c8f5accf655c510fa3dd9a2be293b61d |
| 17 | libqpdf/qpdf/auto_job_schema.hh 81b8d57f05f5125a722912b6ee5e10fc6b02ae68e040108b75f32fc6527c02b1 | 17 | libqpdf/qpdf/auto_job_schema.hh 81b8d57f05f5125a722912b6ee5e10fc6b02ae68e040108b75f32fc6527c02b1 |
| 18 | manual/_ext/qpdf.py 6add6321666031d55ed4aedf7c00e5662bba856dfcd66ccb526563bffefbb580 | 18 | manual/_ext/qpdf.py 6add6321666031d55ed4aedf7c00e5662bba856dfcd66ccb526563bffefbb580 |
| 19 | -manual/cli.rst efbce4b34fefbe1f46fca9c9693af245a7f7e2b63c0853c464569087aa49cb18 | 19 | +manual/cli.rst 02792f51d72029eea5a67f12770a8f46c36e15f8a774e67c85954be16dc7e77f |
| 20 | manual/qpdf.1 0783e8741f21ed28b3d3d2c5e07beb7eef6e227dd82bd0b316a17875f39f1589 | 20 | manual/qpdf.1 0783e8741f21ed28b3d3d2c5e07beb7eef6e227dd82bd0b316a17875f39f1589 |
| 21 | manual/qpdf.1.in 436ecc85d45c4c9e2dbd1725fb7f0177fb627179469f114561adf3cb6cbb677b | 21 | manual/qpdf.1.in 436ecc85d45c4c9e2dbd1725fb7f0177fb627179469f114561adf3cb6cbb677b |
job.yml
| 1 | -# See "HOW TO ADD A COMMAND-LINE ARGUMENT" in README-maintainer. | 1 | +# See "HOW TO ADD A COMMAND-LINE ARGUMENT" in README-developer. |
| 2 | 2 | ||
| 3 | # REMEMBER: if you add an optional_choices or optional_parameter, you | 3 | # REMEMBER: if you add an optional_choices or optional_parameter, you |
| 4 | # have to explicitly remember to implement the overloaded config | 4 | # have to explicitly remember to implement the overloaded config |
manual/cli.rst
| 1 | .. NOTES | 1 | .. NOTES |
| 2 | 2 | ||
| 3 | - See "HOW TO ADD A COMMAND-LINE ARGUMENT" in README-maintainer. | 3 | + See "HOW TO ADD A COMMAND-LINE ARGUMENT" in README-developer. |
| 4 | 4 | ||
| 5 | This file contains text that is used for help file generation. | 5 | This file contains text that is used for help file generation. |
| 6 | Lines that start with the magic comment ".. help topic x: y" | 6 | Lines that start with the magic comment ".. help topic x: y" |
manual/contributing.rst
| @@ -55,9 +55,8 @@ partial summary of the formatting rules: | @@ -55,9 +55,8 @@ partial summary of the formatting rules: | ||
| 55 | - Closing parentheses are attached to the previous material, not not | 55 | - Closing parentheses are attached to the previous material, not not |
| 56 | their own lines. | 56 | their own lines. |
| 57 | 57 | ||
| 58 | -The :file:`README-maintainer` file has a few additional notes that are | ||
| 59 | -probably not important to anyone who is not making deep changes to | ||
| 60 | -qpdf. | 58 | +The :file:`README-developer` file has a few additional notes that are |
| 59 | +probably not important to anyone who is making small changes to qpdf. | ||
| 61 | 60 | ||
| 62 | .. _automated-testing: | 61 | .. _automated-testing: |
| 63 | 62 |
manual/installation.rst
| @@ -312,7 +312,7 @@ Options for Working on qpdf | @@ -312,7 +312,7 @@ Options for Working on qpdf | ||
| 312 | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ | 312 | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
| 313 | 313 | ||
| 314 | ENABLE_COVERAGE | 314 | ENABLE_COVERAGE |
| 315 | - Compile with ``--coverage``. See README-maintainer.md for | 315 | + Compile with ``--coverage``. See README-developer.md for |
| 316 | information about generating coverage reports. | 316 | information about generating coverage reports. |
| 317 | 317 | ||
| 318 | ENABLE_QTC | 318 | ENABLE_QTC |
manual/qpdf-job.rst
| @@ -187,7 +187,7 @@ This section describes some of the design rationale and history behind | @@ -187,7 +187,7 @@ This section describes some of the design rationale and history behind | ||
| 187 | 187 | ||
| 188 | Documentation of ``QPDFJob`` is divided among three places: | 188 | Documentation of ``QPDFJob`` is divided among three places: |
| 189 | 189 | ||
| 190 | -- "HOW TO ADD A COMMAND-LINE ARGUMENT" in :file:`README-maintainer` | 190 | +- "HOW TO ADD A COMMAND-LINE ARGUMENT" in :file:`README-developer` |
| 191 | provides a quick reminder of how to add a command-line argument. | 191 | provides a quick reminder of how to add a command-line argument. |
| 192 | 192 | ||
| 193 | - The source file :file:`generate_auto_job` has a detailed explanation | 193 | - The source file :file:`generate_auto_job` has a detailed explanation |
manual/release-notes.rst
| @@ -276,7 +276,7 @@ more detail. | @@ -276,7 +276,7 @@ more detail. | ||
| 276 | - The file ``.idea/cmake.xml`` has been removed. Instead of | 276 | - The file ``.idea/cmake.xml`` has been removed. Instead of |
| 277 | shipping with some CMake profiles in the CLion-specific | 277 | shipping with some CMake profiles in the CLion-specific |
| 278 | configuration, we now include a ``CMakePresets.json``. There is | 278 | configuration, we now include a ``CMakePresets.json``. There is |
| 279 | - information about using it in ``README-maintainer.md``. For | 279 | + information about using it in ``README-contributor.md``. For |
| 280 | most users, running ``cmake`` in the normal way is fine. | 280 | most users, running ``cmake`` in the normal way is fine. |
| 281 | Suggestions are welcome. None of the official builds use cmake | 281 | Suggestions are welcome. None of the official builds use cmake |
| 282 | presets at the time of initial introduction. | 282 | presets at the time of initial introduction. |
| @@ -627,7 +627,7 @@ more detail. | @@ -627,7 +627,7 @@ more detail. | ||
| 627 | alternative ``zlib`` implementation. There are no dependencies | 627 | alternative ``zlib`` implementation. There are no dependencies |
| 628 | anywhere in the qpdf test suite on any particular ``zlib`` | 628 | anywhere in the qpdf test suite on any particular ``zlib`` |
| 629 | output. Consult the ``ZLIB COMPATIBILITY`` section of | 629 | output. Consult the ``ZLIB COMPATIBILITY`` section of |
| 630 | - ``README-maintainer.md`` for a detailed explanation of how to | 630 | + ``README-contributor.md`` for a detailed explanation of how to |
| 631 | maintain this. | 631 | maintain this. |
| 632 | 632 | ||
| 633 | - The official Windows installers now offers to modify ``PATH`` | 633 | - The official Windows installers now offers to modify ``PATH`` |
qpdf/qpdf.cc
| @@ -31,7 +31,7 @@ realmain(int argc, char* argv[]) | @@ -31,7 +31,7 @@ realmain(int argc, char* argv[]) | ||
| 31 | 31 | ||
| 32 | QPDFJob j; | 32 | QPDFJob j; |
| 33 | try { | 33 | try { |
| 34 | - // See "HOW TO ADD A COMMAND-LINE ARGUMENT" in README-maintainer. | 34 | + // See "HOW TO ADD A COMMAND-LINE ARGUMENT" in README-developer. |
| 35 | j.initializeFromArgv(argv); | 35 | j.initializeFromArgv(argv); |
| 36 | j.run(); | 36 | j.run(); |
| 37 | } catch (QPDFUsage& e) { | 37 | } catch (QPDFUsage& e) { |