Commit deeface146800bfe8b91986ccd98464cac833236
1 parent
cfafac8d
Add automated test for shell wildcard expansion
Wildcard expansion is different in Windows from non-Windows and sometimes requires special link options to work. Add tests that fail if we link incorrectly.
Showing
5 changed files
with
85 additions
and
43 deletions
.github/workflows/main.yml
| @@ -41,7 +41,6 @@ jobs: | @@ -41,7 +41,6 @@ jobs: | ||
| 41 | name: distribution | 41 | name: distribution |
| 42 | path: distribution | 42 | path: distribution |
| 43 | Windows: | 43 | Windows: |
| 44 | - # If updating this, see note in TODO about MSVC wildcard expansion. | ||
| 45 | runs-on: windows-2019 | 44 | runs-on: windows-2019 |
| 46 | needs: Distfiles | 45 | needs: Distfiles |
| 47 | strategy: | 46 | strategy: |
TODO
| @@ -4,8 +4,7 @@ Candidates for upcoming release | @@ -4,8 +4,7 @@ Candidates for upcoming release | ||
| 4 | * Easy build/test | 4 | * Easy build/test |
| 5 | * #460: potential malware in fuzzer seed corpus | 5 | * #460: potential malware in fuzzer seed corpus |
| 6 | * Consider building workflow on a schedule to detect build rot. This | 6 | * Consider building workflow on a schedule to detect build rot. This |
| 7 | - may enable safe use of *-latest especially if Windows wildcard is | ||
| 8 | - testable. | 7 | + should enable safe use of *-latest on runners. |
| 9 | 8 | ||
| 10 | * Fuzz crashes | 9 | * Fuzz crashes |
| 11 | * See "New" below | 10 | * See "New" below |
| @@ -29,9 +28,6 @@ Candidates for upcoming release | @@ -29,9 +28,6 @@ Candidates for upcoming release | ||
| 29 | because of changes in the build environment, library dependencies, | 28 | because of changes in the build environment, library dependencies, |
| 30 | compiler upgrades, etc. | 29 | compiler upgrades, etc. |
| 31 | 30 | ||
| 32 | -* Find a way to deal with MSVC wildcard expansion, even if it requires | ||
| 33 | - creating a separate step or adding code to build-windows.bat. | ||
| 34 | - | ||
| 35 | * See if we can work in Windows Build/External Libraries (below) | 31 | * See if we can work in Windows Build/External Libraries (below) |
| 36 | 32 | ||
| 37 | * Remember to check work `qpdf` project for private issues | 33 | * Remember to check work `qpdf` project for private issues |
| @@ -220,34 +216,6 @@ Page splitting/merging | @@ -220,34 +216,6 @@ Page splitting/merging | ||
| 220 | 216 | ||
| 221 | * Form fields: should be similar to outlines. | 217 | * Form fields: should be similar to outlines. |
| 222 | 218 | ||
| 223 | -MSVC Wildcard Expansion | ||
| 224 | -======================= | ||
| 225 | - | ||
| 226 | -(This section is referenced in azure_pipelines.yml and | ||
| 227 | -.github/workflows/main.yml.) | ||
| 228 | - | ||
| 229 | -The qpdf executable built with msvc is linked with setargv.obj or | ||
| 230 | -wsetargv.obj so that wildcard expansion works. It doesn't work exactly | ||
| 231 | -the way a UNIX system would work in that the program itself does the | ||
| 232 | -expansion (rather than the shell), which means that invoking qpdf.exe | ||
| 233 | -as built by msvc will expand "*.pdf" just as it will expand *.pdf. In | ||
| 234 | -some earlier versions, wildcard expansion didn't work with the msvc | ||
| 235 | -executable. The way to make this work appears to be different in some | ||
| 236 | -versions of MSVC than in others. As such, if upgrading MSVC or | ||
| 237 | -changing the build environment, the wildcard expansion behavior of the | ||
| 238 | -qpdf executable in Windows should be retested manually. | ||
| 239 | - | ||
| 240 | -Unfortunately, there is no automated test for wildcard expansion with | ||
| 241 | -MSVC because I can't figure out how to prevent qtest from expanding | ||
| 242 | -the wildcards before passing them in, and explicitly running "cmd /c | ||
| 243 | -..." from qtest doesn't seem to work in Azure Pipelines (haven't | ||
| 244 | -attempted in GitHub Actions), though I can make it work locally. | ||
| 245 | - | ||
| 246 | -Ideally, we should figure out a way to test this in CI by having a | ||
| 247 | -test that fails if wildcard expansion is broken. In the absence of | ||
| 248 | -this, it will be necessary to test the behavior manually in both mingw | ||
| 249 | -and msvc when run from cmd and from msys bash. | ||
| 250 | - | ||
| 251 | Performance | 219 | Performance |
| 252 | =========== | 220 | =========== |
| 253 | 221 |
qpdf/build.mk
| @@ -8,6 +8,7 @@ BINS_qpdf = \ | @@ -8,6 +8,7 @@ BINS_qpdf = \ | ||
| 8 | test_pdf_doc_encoding \ | 8 | test_pdf_doc_encoding \ |
| 9 | test_pdf_unicode \ | 9 | test_pdf_unicode \ |
| 10 | test_renumber \ | 10 | test_renumber \ |
| 11 | + test_shell_glob \ | ||
| 11 | test_tokenizer \ | 12 | test_tokenizer \ |
| 12 | test_unicode_filenames \ | 13 | test_unicode_filenames \ |
| 13 | test_xref | 14 | test_xref |
| @@ -23,15 +24,14 @@ TC_SRCS_qpdf = $(wildcard libqpdf/*.cc) $(wildcard qpdf/*.cc) | @@ -23,15 +24,14 @@ TC_SRCS_qpdf = $(wildcard libqpdf/*.cc) $(wildcard qpdf/*.cc) | ||
| 23 | 24 | ||
| 24 | # ----- | 25 | # ----- |
| 25 | 26 | ||
| 26 | -XCXXFLAGS_qpdf_qpdf := $(WINDOWS_WMAIN_COMPILE) | ||
| 27 | -XLDFLAGS_qpdf_qpdf := $(WINDOWS_WMAIN_LINK) | ||
| 28 | -XLINK_FLAGS_qpdf_qpdf := $(WINDOWS_WMAIN_XLINK_FLAGS) | ||
| 29 | -XCXXFLAGS_qpdf_test_unicode_filenames := $(WINDOWS_WMAIN_COMPILE) | ||
| 30 | -XLDFLAGS_qpdf_test_unicode_filenames := $(WINDOWS_WMAIN_LINK) | ||
| 31 | -XLINK_FLAGS_qpdf_test_unicode_filenames := $(WINDOWS_WMAIN_XLINK_FLAGS) | ||
| 32 | -XCXXFLAGS_qpdf_fix-qdf := $(WINDOWS_WMAIN_COMPILE) | ||
| 33 | -XLDFLAGS_qpdf_fix-qdf := $(WINDOWS_WMAIN_LINK) | ||
| 34 | -XLINK_FLAGS_qpdf_fix-qdf := $(WINDOWS_WMAIN_XLINK_FLAGS) | 27 | +define use_wmain |
| 28 | + XCXXFLAGS_qpdf_$(1) := $(WINDOWS_WMAIN_COMPILE) | ||
| 29 | + XLDFLAGS_qpdf_$(1) := $(WINDOWS_WMAIN_LINK) | ||
| 30 | + XLINK_FLAGS_qpdf_$(1) := $(WINDOWS_WMAIN_XLINK_FLAGS) | ||
| 31 | +endef | ||
| 32 | + | ||
| 33 | +$(foreach B,qpdf test_unicode_filenames fix-qdf test_shell_glob,\ | ||
| 34 | + $(eval $(call use_wmain,$(B)))) | ||
| 35 | 35 | ||
| 36 | $(foreach B,$(BINS_qpdf),$(eval \ | 36 | $(foreach B,$(BINS_qpdf),$(eval \ |
| 37 | OBJS_$(B) = $(call src_to_obj,qpdf/$(B).cc))) | 37 | OBJS_$(B) = $(call src_to_obj,qpdf/$(B).cc))) |
qpdf/qtest/qpdf.test
| @@ -191,6 +191,17 @@ foreach my $d (['auto-ü', 1], ['auto-öπ', 2]) | @@ -191,6 +191,17 @@ foreach my $d (['auto-ü', 1], ['auto-öπ', 2]) | ||
| 191 | 191 | ||
| 192 | show_ntests(); | 192 | show_ntests(); |
| 193 | # ---------- | 193 | # ---------- |
| 194 | +$td->notify("--- Windows shell globbing ---"); | ||
| 195 | + | ||
| 196 | +$td->runtest("shell wildcard expansion", | ||
| 197 | + {$td->COMMAND => "test_shell_glob 'good*.pdf'"}, | ||
| 198 | + {$td->STRING => "PASSED\n", $td->EXIT_STATUS => 0}, | ||
| 199 | + $td->NORMALIZE_NEWLINES); | ||
| 200 | + | ||
| 201 | +$n_tests += 1; | ||
| 202 | + | ||
| 203 | +show_ntests(); | ||
| 204 | +# ---------- | ||
| 194 | $td->notify("--- Replace Input ---"); | 205 | $td->notify("--- Replace Input ---"); |
| 195 | $n_tests += 8; | 206 | $n_tests += 8; |
| 196 | 207 |
qpdf/test_shell_glob.cc
0 → 100644
| 1 | +#include <iostream> | ||
| 2 | +#include <cstring> | ||
| 3 | +#include <qpdf/QUtil.hh> | ||
| 4 | + | ||
| 5 | +int realmain(int argc, char* argv[]) | ||
| 6 | +{ | ||
| 7 | + // In Windows, shell globbing is handled by the runtime, so | ||
| 8 | + // passing '*' as argument results in wildcard expansion. In | ||
| 9 | + // non-Windows, globbing is done by the shell, so passing '*' | ||
| 10 | + // shows up as '*'. In Windows with MSVC, it is necessary to link | ||
| 11 | + // a certain way for this to work. To test this, we invoke this | ||
| 12 | + // program with a single quoted argument containing a shell glob | ||
| 13 | + // expression. In Windows, we expect to see multiple arguments, | ||
| 14 | + // none of which contain '*'. Otherwise, we expected to see the | ||
| 15 | + // exact glob string that was passed in. The effectiveness of this | ||
| 16 | + // test was exercised by manually breaking the link options for | ||
| 17 | + // msvc and seeing that the test fails under that condition. | ||
| 18 | + | ||
| 19 | + bool found_star = false; | ||
| 20 | + for (int i = 1; i < argc; ++i) | ||
| 21 | + { | ||
| 22 | + if (strchr(argv[i], '*') != nullptr) | ||
| 23 | + { | ||
| 24 | + found_star = true; | ||
| 25 | + break; | ||
| 26 | + } | ||
| 27 | + } | ||
| 28 | +#ifdef _WIN32 | ||
| 29 | + bool passed = ((! found_star) && (argc > 2)); | ||
| 30 | +#else | ||
| 31 | + bool passed = (found_star && (argc == 2)); | ||
| 32 | +#endif | ||
| 33 | + if (passed) | ||
| 34 | + { | ||
| 35 | + std::cout << "PASSED" << std::endl; | ||
| 36 | + } | ||
| 37 | + else | ||
| 38 | + { | ||
| 39 | + std::cout << "FAILED" << std::endl; | ||
| 40 | + for (int i = 1; i < argc; ++i) | ||
| 41 | + { | ||
| 42 | + std::cout << argv[i] << std::endl; | ||
| 43 | + } | ||
| 44 | + } | ||
| 45 | + return 0; | ||
| 46 | +} | ||
| 47 | + | ||
| 48 | + | ||
| 49 | +#ifdef WINDOWS_WMAIN | ||
| 50 | + | ||
| 51 | +extern "C" | ||
| 52 | +int wmain(int argc, wchar_t* argv[]) | ||
| 53 | +{ | ||
| 54 | + return QUtil::call_main_from_wmain(argc, argv, realmain); | ||
| 55 | +} | ||
| 56 | + | ||
| 57 | +#else | ||
| 58 | + | ||
| 59 | +int main(int argc, char* argv[]) | ||
| 60 | +{ | ||
| 61 | + return realmain(argc, argv); | ||
| 62 | +} | ||
| 63 | + | ||
| 64 | +#endif |