diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 1933d371..441e5b23 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -41,7 +41,6 @@ jobs: name: distribution path: distribution Windows: - # If updating this, see note in TODO about MSVC wildcard expansion. runs-on: windows-2019 needs: Distfiles strategy: diff --git a/TODO b/TODO index 2e76e5f8..f0d5f8a4 100644 --- a/TODO +++ b/TODO @@ -4,8 +4,7 @@ Candidates for upcoming release * Easy build/test * #460: potential malware in fuzzer seed corpus * Consider building workflow on a schedule to detect build rot. This - may enable safe use of *-latest especially if Windows wildcard is - testable. + should enable safe use of *-latest on runners. * Fuzz crashes * See "New" below @@ -29,9 +28,6 @@ Candidates for upcoming release because of changes in the build environment, library dependencies, compiler upgrades, etc. -* Find a way to deal with MSVC wildcard expansion, even if it requires - creating a separate step or adding code to build-windows.bat. - * See if we can work in Windows Build/External Libraries (below) * Remember to check work `qpdf` project for private issues @@ -220,34 +216,6 @@ Page splitting/merging * Form fields: should be similar to outlines. -MSVC Wildcard Expansion -======================= - -(This section is referenced in azure_pipelines.yml and -.github/workflows/main.yml.) - -The qpdf executable built with msvc is linked with setargv.obj or -wsetargv.obj so that wildcard expansion works. It doesn't work exactly -the way a UNIX system would work in that the program itself does the -expansion (rather than the shell), which means that invoking qpdf.exe -as built by msvc will expand "*.pdf" just as it will expand *.pdf. In -some earlier versions, wildcard expansion didn't work with the msvc -executable. The way to make this work appears to be different in some -versions of MSVC than in others. As such, if upgrading MSVC or -changing the build environment, the wildcard expansion behavior of the -qpdf executable in Windows should be retested manually. - -Unfortunately, there is no automated test for wildcard expansion with -MSVC because I can't figure out how to prevent qtest from expanding -the wildcards before passing them in, and explicitly running "cmd /c -..." from qtest doesn't seem to work in Azure Pipelines (haven't -attempted in GitHub Actions), though I can make it work locally. - -Ideally, we should figure out a way to test this in CI by having a -test that fails if wildcard expansion is broken. In the absence of -this, it will be necessary to test the behavior manually in both mingw -and msvc when run from cmd and from msys bash. - Performance =========== diff --git a/qpdf/build.mk b/qpdf/build.mk index 1bab58b7..bf1f52f8 100644 --- a/qpdf/build.mk +++ b/qpdf/build.mk @@ -8,6 +8,7 @@ BINS_qpdf = \ test_pdf_doc_encoding \ test_pdf_unicode \ test_renumber \ + test_shell_glob \ test_tokenizer \ test_unicode_filenames \ test_xref @@ -23,15 +24,14 @@ TC_SRCS_qpdf = $(wildcard libqpdf/*.cc) $(wildcard qpdf/*.cc) # ----- -XCXXFLAGS_qpdf_qpdf := $(WINDOWS_WMAIN_COMPILE) -XLDFLAGS_qpdf_qpdf := $(WINDOWS_WMAIN_LINK) -XLINK_FLAGS_qpdf_qpdf := $(WINDOWS_WMAIN_XLINK_FLAGS) -XCXXFLAGS_qpdf_test_unicode_filenames := $(WINDOWS_WMAIN_COMPILE) -XLDFLAGS_qpdf_test_unicode_filenames := $(WINDOWS_WMAIN_LINK) -XLINK_FLAGS_qpdf_test_unicode_filenames := $(WINDOWS_WMAIN_XLINK_FLAGS) -XCXXFLAGS_qpdf_fix-qdf := $(WINDOWS_WMAIN_COMPILE) -XLDFLAGS_qpdf_fix-qdf := $(WINDOWS_WMAIN_LINK) -XLINK_FLAGS_qpdf_fix-qdf := $(WINDOWS_WMAIN_XLINK_FLAGS) +define use_wmain + XCXXFLAGS_qpdf_$(1) := $(WINDOWS_WMAIN_COMPILE) + XLDFLAGS_qpdf_$(1) := $(WINDOWS_WMAIN_LINK) + XLINK_FLAGS_qpdf_$(1) := $(WINDOWS_WMAIN_XLINK_FLAGS) +endef + +$(foreach B,qpdf test_unicode_filenames fix-qdf test_shell_glob,\ + $(eval $(call use_wmain,$(B)))) $(foreach B,$(BINS_qpdf),$(eval \ OBJS_$(B) = $(call src_to_obj,qpdf/$(B).cc))) diff --git a/qpdf/qtest/qpdf.test b/qpdf/qtest/qpdf.test index 71f55636..c2a38fa3 100644 --- a/qpdf/qtest/qpdf.test +++ b/qpdf/qtest/qpdf.test @@ -189,6 +189,17 @@ foreach my $d (['auto-ü', 1], ['auto-öπ', 2]) $td->NORMALIZE_NEWLINES); } +show_ntests(); +# ---------- +$td->notify("--- Windows shell globbing ---"); + +$td->runtest("shell wildcard expansion", + {$td->COMMAND => "test_shell_glob 'good*.pdf'"}, + {$td->STRING => "PASSED\n", $td->EXIT_STATUS => 0}, + $td->NORMALIZE_NEWLINES); + +$n_tests += 1; + show_ntests(); # ---------- $td->notify("--- Replace Input ---"); diff --git a/qpdf/test_shell_glob.cc b/qpdf/test_shell_glob.cc new file mode 100644 index 00000000..34582197 --- /dev/null +++ b/qpdf/test_shell_glob.cc @@ -0,0 +1,64 @@ +#include +#include +#include + +int realmain(int argc, char* argv[]) +{ + // In Windows, shell globbing is handled by the runtime, so + // passing '*' as argument results in wildcard expansion. In + // non-Windows, globbing is done by the shell, so passing '*' + // shows up as '*'. In Windows with MSVC, it is necessary to link + // a certain way for this to work. To test this, we invoke this + // program with a single quoted argument containing a shell glob + // expression. In Windows, we expect to see multiple arguments, + // none of which contain '*'. Otherwise, we expected to see the + // exact glob string that was passed in. The effectiveness of this + // test was exercised by manually breaking the link options for + // msvc and seeing that the test fails under that condition. + + bool found_star = false; + for (int i = 1; i < argc; ++i) + { + if (strchr(argv[i], '*') != nullptr) + { + found_star = true; + break; + } + } +#ifdef _WIN32 + bool passed = ((! found_star) && (argc > 2)); +#else + bool passed = (found_star && (argc == 2)); +#endif + if (passed) + { + std::cout << "PASSED" << std::endl; + } + else + { + std::cout << "FAILED" << std::endl; + for (int i = 1; i < argc; ++i) + { + std::cout << argv[i] << std::endl; + } + } + return 0; +} + + +#ifdef WINDOWS_WMAIN + +extern "C" +int wmain(int argc, wchar_t* argv[]) +{ + return QUtil::call_main_from_wmain(argc, argv, realmain); +} + +#else + +int main(int argc, char* argv[]) +{ + return realmain(argc, argv); +} + +#endif