Link with setargv or wsetargv with MSVC (fixes #224)

For wildcard expansion to work properly with the msvc binary, it is necessary to link with setargv.obj or wsetargv.obj, depending on whether wmain is in use.
2019-11-09 15:01:15 -05:00 · 2019-11-09 15:01:15 -05:00 · 07da0039d3
parent a58918bb87
commit 07da0039d3
12 changed files with 76 additions and 10 deletions
--- a/10
+++ b/10
@ -1,5 +1,15 @@
 2019-11-09  Jay Berkenbilt  <ejb@ql.org>

+	* Improve behavior of wildcard expansion for msvc executable when
+	run from the Windows cmd.exe shell. The behavior is not identical
+	to UNIX or to running the mingw executable in that, for msvc,
+	wildcard expansion is performed transparently by qpdf.exe itself.
+	That means that *.pdf and "*.pdf*" are interpreted identically. If
+	you need to specify the name of a file that has a * or ? in its
+	actual filename on the Windows CLI with the msvc-built executable,
+	you can write the name of the file in another file and use the
+	@file syntax to prevent expansion. Fixes #224.
+
 	* When reading /P from the encryption dictionary, use static_cast
 	instead of QIntC to convert the value to a signed integer. The
 	value of /P is a bit field, and PDF files have been found in the
--- a/27
+++ b/27
@ -132,6 +132,33 @@ Page splitting/merging

 * Form fields: should be similar to outlines.

+MSVC Wildcard Expansion
+=======================
+
+(This section is referenced in azure_pipelines.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, 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.
+
 General
 =======

--- a/autoconf.mk.in
+++ b/autoconf.mk.in
@ -47,6 +47,8 @@ IS_32BIT=@IS_32BIT@
 WINDOWS_WORDSIZE=@WINDOWS_WORDSIZE@
 WINDOWS_WMAIN_COMPILE=@WINDOWS_WMAIN_COMPILE@
 WINDOWS_WMAIN_LINK=@WINDOWS_WMAIN_LINK@
+WINDOWS_WMAIN_XLINK_FLAGS=@WINDOWS_WMAIN_XLINK_FLAGS@
+WINDOWS_MAIN_XLINK_FLAGS=@WINDOWS_MAIN_XLINK_FLAGS@
 SHOW_FAILED_TEST_OUTPUT=@SHOW_FAILED_TEST_OUTPUT@
 # Allow environment variable to override
 QPDF_LARGE_FILE_TEST_PATH?=@QPDF_LARGE_FILE_TEST_PATH@
--- a/autofiles.sums
+++ b/autofiles.sums
@ -1,4 +1,4 @@
-97f3ed3cd8b491f0ceeb57baa40f4ed9c4be188692da1d13c93ef318c45cc4ae  configure.ac
+50057c548d9af98d7b070a8a7716352a777b1f52e7ba44719f94ede65fc27cee  configure.ac
 d3f9ee6f6f0846888d9a10fd3dad2e4b1258be84205426cf04d7cef02d61dad7  aclocal.m4
 2e4cd495837be1b8454a4d8aef541b000988634be89d9c05a9cf5de67dffef5e  libqpdf/qpdf/qpdf-config.h.in
 5297971a0ef90bcd5563eb3f7127a032bb76d3ae2af7258bf13479caf8983a60  m4/ax_cxx_compile_stdcxx.m4
--- a/azure-pipelines.yml
+++ b/azure-pipelines.yml
@ -23,6 +23,7 @@ jobs:
      buildPlatform: Linux
 - job: Windows
  pool:
+    # If updating this, see note in TODO about MSVC wildcard expansion.
    vmImage: windows-2019
  strategy:
    matrix:
--- a/9
+++ b/9
@ -647,6 +647,8 @@ USE_CRYPTO_GNUTLS
 pc_gnutls_LIBS
 pc_gnutls_CFLAGS
 USE_CRYPTO_NATIVE
+WINDOWS_MAIN_XLINK_FLAGS
+WINDOWS_WMAIN_XLINK_FLAGS
 CXXWFLAGS
 WFLAGS
 BUILDRULES
@ -17595,6 +17597,13 @@ fi



+if test "$BUILDRULES" = "msvc"; then
+  WINDOWS_WMAIN_XLINK_FLAGS="-link wsetargv.obj"
+  WINDOWS_MAIN_XLINK_FLAGS="-link setargv.obj"
+fi
+
+
+
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking for whether to use implicit crypto" >&5
 $as_echo_n "checking for whether to use implicit crypto... " >&6; }
 # Check whether --enable-implicit-crypto was given.
--- a/configure.ac
+++ b/configure.ac
@ -465,6 +465,13 @@ else
   AC_MSG_RESULT(no)
 fi

+AC_SUBST(WINDOWS_WMAIN_XLINK_FLAGS)
+AC_SUBST(WINDOWS_MAIN_XLINK_FLAGS)
+if test "$BUILDRULES" = "msvc"; then
+  WINDOWS_WMAIN_XLINK_FLAGS="-link wsetargv.obj"
+  WINDOWS_MAIN_XLINK_FLAGS="-link setargv.obj"
+fi
+
 dnl BEGIN CRYPTO

 dnl By default, we build in support for every crypto provider that we
--- a/make/libtool.mk
+++ b/make/libtool.mk
@ -98,10 +98,10 @@ define makelib
 		 -rpath $(libdir) -version-info $(5):$(6):$(7) -no-undefined
 endef

-#                       1    2      3       4
-# Usage: $(call makebin,objs,binary,ldflags,libs)
+#                       1    2      3       4    5
+# Usage: $(call makebin,objs,binary,ldflags,libs,xlinkflags)
 define makebin
-	$(LIBTOOL) --mode=link $(CXX) $(CXXFLAGS) $(1) -o $(2) $(3) $(4)
+	$(LIBTOOL) --mode=link $(CXX) $(CXXFLAGS) $(5) $(1) -o $(2) $(3) $(4)
 endef

 # Install target
--- a/make/mingw.mk
+++ b/make/mingw.mk
@ -67,10 +67,10 @@ define makelib
 		$(1) $(3) $(4)
 endef

-#                       1    2      3       4
-# Usage: $(call makebin,objs,binary,ldflags,libs)
+#                       1    2      3       4    5
+# Usage: $(call makebin,objs,binary,ldflags,libs,xlinkflags)
 define makebin
-	$(CXX) $(CXXFLAGS) $(1) -o $(2) $(3) $(4)
+	$(CXX) $(CXXFLAGS) $(5) $(1) -o $(2) $(3) $(4)
 endef

 # Install target
--- a/make/msvc.mk
+++ b/make/msvc.mk
@ -76,10 +76,11 @@ define makelib
 	mv $(basename $(2))$(shell expr $(5) - $(7)).lib $(2)
 endef

-#                       1    2      3       4
-# Usage: $(call makebin,objs,binary,ldflags,libs)
+#                       1    2      3       4    5
+# Usage: $(call makebin,objs,binary,ldflags,libs,xlinkflags)
 define makebin
 	cl -nologo -O2 -Zi -Gy -EHsc -MD $(1) \
+		$(if $(5),$(5),$(WINDOWS_MAIN_XLINK_FLAGS)) \
 		-link -SUBSYSTEM:CONSOLE,5.01 -incremental:no -OUT:$(2) \
 		$(foreach L,$(subst -L,,$(3)),-LIBPATH:$(L)) \
 		$(foreach L,$(subst -l,,$(4)),$(L).lib)
--- a/manual/qpdf-manual.xml
+++ b/manual/qpdf-manual.xml
@ -4685,6 +4685,13 @@ print "\n";
          or odd pages from among the pages specified in the range.
         </para>
        </listitem>
+        <listitem>
+         <para>
+          Improve shell wildcard expansion behavior
+          (<literal>*</literal> and <literal>?</literal>) of the
+          <command>qpdf.exe</command> as built my MSVC.
+         </para>
+        </listitem>
       </itemizedlist>
      </listitem>
     </itemizedlist>
--- a/qpdf/build.mk
+++ b/qpdf/build.mk
@ -26,6 +26,8 @@ XCXXFLAGS_qpdf_qpdf := $(WINDOWS_WMAIN_COMPILE)
 XLDFLAGS_qpdf_qpdf := $(WINDOWS_WMAIN_LINK)
 XCXXFLAGS_qpdf_test_unicode_filenames := $(WINDOWS_WMAIN_COMPILE)
 XLDFLAGS_qpdf_test_unicode_filenames := $(WINDOWS_WMAIN_LINK)
+XLINK_FLAGS_qpdf_qpdf := $(WINDOWS_WMAIN_XLINK_FLAGS)
+XLINK_FLAGS_qpdf_test_unicode_filenames := $(WINDOWS_WMAIN_XLINK_FLAGS)

 $(foreach B,$(BINS_qpdf),$(eval \
  OBJS_$(B) = $(call src_to_obj,qpdf/$(B).cc)))
@ -46,4 +48,4 @@ $(foreach B,$(CBINS_qpdf),$(eval \

 $(foreach B,$(BINS_qpdf) $(CBINS_qpdf),$(eval \
  qpdf/$(OUTPUT_DIR)/$(call binname,$(B)): $(OBJS_$(B)) ; \
-	$(call makebin,$(OBJS_$(B)),$$@,$(LDFLAGS_libqpdf) $(LDFLAGS) $(XLDFLAGS_qpdf_$(B)),$(LIBS_libqpdf) $(LIBS))))
+	$(call makebin,$(OBJS_$(B)),$$@,$(LDFLAGS_libqpdf) $(LDFLAGS) $(XLDFLAGS_qpdf_$(B)),$(LIBS_libqpdf) $(LIBS),$(XLINK_FLAGS_qpdf_$(B)))))