Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[openssl] Fix build with "C:\Program Files\<...>\cl.exe" #43089

Merged
merged 1 commit into from
Jan 8, 2025

Conversation

dg0yt
Copy link
Contributor

@dg0yt dg0yt commented Jan 4, 2025

This fixes the long-standing annoyance that port openssl fails to build with (some) installations of Visual Studio to paths including the space character, also affecting the default location C:\Program Files\Visual Studio\....

The port passes the absolute path of CC and the CFLAGS as detected from the CMake toolchain.
This works for all normal build rules, but not for the invocation of a perl scripts which is meant to write that information into crypto/buildinf.h which is created via this rule:

$(PERL) <SRCDIR>/util/mkbuildinf.pl "$(CC) $(LIB_CFLAGS) $(CPPFLAGS_Q)" "$(PLATFORM)" > $@
  • The rule doesn't properly handle quotes in CC or flags (at least for Windows host), so perl may see multiple arguments when it expects to see exactly two arguments.
  • mkbuildinf.pl doesn't consider the backslash in filepaths like CC when constructing a C string.

The new patch

  • handles multiple arguments, assuming only that the last one is the platform.
  • quotes all backslashes for C string construction.

NB the full path of the compiler is written as build information into binary aritfacts. If this isn't desired, a different approach must be taken.

Resolves #42740.
Resolves #41751.
Resolves #39912.
Resolves #38723.
Resolves #36175.
Related:

@dg0yt
Copy link
Contributor Author

dg0yt commented Jan 4, 2025

Upstream: openssl/openssl#26315.

@jimwang118 jimwang118 self-assigned this Jan 6, 2025
@jimwang118 jimwang118 added the category:port-bug The issue is with a library, which is something the port should already support label Jan 6, 2025
@jimwang118
Copy link
Contributor

Compile test pass with following triplets:

x86-windows
x64-windows
x86-windows-static

@jimwang118 jimwang118 added the info:reviewed Pull Request changes follow basic guidelines label Jan 6, 2025
@JavierMatosD JavierMatosD added the requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. label Jan 7, 2025
@dg0yt
Copy link
Contributor Author

dg0yt commented Jan 8, 2025

Effect:

@@ -10,7 +10,7 @@
  * https://www.openssl.org/source/license.html
  */

-#define PLATFORM "platform: Files\Microsoft"
+#define PLATFORM "platform: VC-WIN64A"
 #define DATE "built on: Fri Jan  3 21:35:31 2025 UTC"

 /*
@@ -19,6 +19,32 @@
  * literal
  */
 static const char compiler_flags[] = {
-    'c','o','m','p','i','l','e','r',':',' ','C',':','\\','P','r','o',
-    'g','r','a','m','\0'
+    'c','o','m','p','i','l','e','r',':',' ','C',':','\\','\\','P','r',
+    'o','g','r','a','m',' ','F','i','l','e','s','\\','\\','M','i','c',
+    'r','o','s','o','f','t',' ','V','i','s','u','a','l',' ','S','t',
+    'u','d','i','o','\\','\\','2','0','2','2','\\','\\','C','o','m','m',
+    'u','n','i','t','y','\\','\\','V','C','\\','\\','T','o','o','l','s',
+    '\\','\\','M','S','V','C','\\','\\','1','4','.','4','2','.','3','4',
+    '4','3','3','\\','\\','b','i','n','\\','\\','H','o','s','t','x','6',
+    '4','\\','\\','x','6','4','\\','\\','c','l','.','e','x','e',' ','/',
+    'Z','i',' ','/','F','d','o','s','s','l','_','s','t','a','t','i',
+    'c','.','p','d','b',' ','/','G','s','0',' ','/','G','F',' ','/',
+    'G','y',' ','/','M','D',' ','-','n','o','l','o','g','o',' ','-',
+    'D','W','I','N','3','2',' ','-','D','_','W','I','N','D','O','W',
+    'S',' ','-','u','t','f','-','8',' ','-','M','P',' ',' ','-','M',
+    'D',' ','-','O','2',' ','-','O','i',' ','-','G','y',' ','-','D',
+    'N','D','E','B','U','G',' ','-','Z','7',' ','-','D','L','_','E',
+    'N','D','I','A','N',' ','-','D','O','P','E','N','S','S','L','_',
+    'P','I','C',' ','-','D','"','O','P','E','N','S','S','L','_','B',
+    'U','I','L','D','I','N','G','_','O','P','E','N','S','S','L','"',
+    ' ','-','D','"','O','P','E','N','S','S','L','_','S','Y','S','_',
+    'W','I','N','3','2','"',' ','-','D','"','W','I','N','3','2','_',
+    'L','E','A','N','_','A','N','D','_','M','E','A','N','"',' ','-',
+    'D','"','U','N','I','C','O','D','E','"',' ','-','D','"','_','U',
+    'N','I','C','O','D','E','"',' ','-','D','"','_','C','R','T','_',
+    'S','E','C','U','R','E','_','N','O','_','D','E','P','R','E','C',
+    'A','T','E','"',' ','-','D','"','_','W','I','N','S','O','C','K',
+    '_','D','E','P','R','E','C','A','T','E','D','_','N','O','_','W',
+    'A','R','N','I','N','G','S','"',' ','-','D','"','N','D','E','B',
+    'U','G','"','\0'
 };

@JavierMatosD JavierMatosD merged commit f0a401b into microsoft:master Jan 8, 2025
17 checks passed
@dg0yt dg0yt deleted the openssl branch January 8, 2025 16:58
@jimwang118 jimwang118 removed the requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. label Jan 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
3 participants