Adding rule to build Windows .rc files #581

Overhatted · 2024-02-26T20:54:33Z

Adds the ability to compile resource files: https://learn.microsoft.com/en-us/windows/win32/menurc/about-resource-files

More details are in the commit messages.

The rc.exe and cvtres.exe are, respectively, called "Windows Resource Compiler" and "Windows Resource to Object Converter". I made finding rc.exe in vswhere.py optional since it is part of the Windows SDK which is already being handled as optional in that script. Although I'm not sure the optional part is currently working.

…soft.com/en-us/windows/win32/menurc/about-resource-files The "Windows Resource Compiler" compiles .rc files into .res files which the "Windows Resource to Object Converter" then converts into .obj files, to be used by the linker. The .res files can also be passed directly to the linker which will call the "Windows Resource to Object Converter" in the same directory to convert the file. Since with Buck2 we want explicit dependencies as much as possible I opted to convert the resource file to an .obj file explicitly in the windows_resouce rule.

ndmitchell · 2024-03-03T17:03:56Z

CC @lmvasquezg @KapJI as Windows experts

KapJI

Overall looks good but please add a end to end test which uses windows_resource.

KapJI · 2024-03-04T15:28:24Z

prelude/toolchains/msvc/vswhere.py

@@ -81,6 +86,9 @@ def find_with_vswhere_exe():
        reverse=True,
    )

+    vc_exe_names = "cl.exe", "cvtres.exe", "lib.exe", "ml64.exe", "link.exe"


Semantically these are lists, not tuples.

When I do:

vc_exe_names = "cl.exe", "cvtres.exe", "lib.exe", "ml64.exe", "link.exe" type(vc_exe_names)

I get tuple.
Is it some obscure Python feature that it is actually a list? Unless I'm misunderstanding what you mean.

I means it should be a list instead of tuple.

Of course, you're right, semantically they are lists

prelude/toolchains/msvc/vswhere.py

prelude/cxx/cxx_toolchain.bzl

prelude/cxx/windows_resource.bzl

prelude/decls/cxx_rules.bzl

KapJI · 2024-03-04T22:39:55Z

It seems our test suite isn't open sourced, I can add test later myself. If you can, please post some basic example which I can turn into test.

Co-authored-by: Ruslan Sayfutdinov <ruslan@sayfutdinov.com>

prelude/cxx/windows_resource.bzl

Overhatted · 2024-03-06T21:12:35Z

My sample test:
BUCK:

# A rule that includes a single .rc file and compiles it into an object file.
windows_resource(
    name = "resources",
    srcs = [
        "resources.rc",
    ],
)
# A rule that links against the above windows_resource rule.
cxx_binary(
    name = "app",
    srcs = [
        "main.cpp",
    ],
    deps = [
        ":resources"
    ],
    linker_flags = [
        "User32.lib",
    ],
)

main.cpp:

#include <iostream>
#include <Windows.h>

int main(int argc, const char* argv[])
{
	std::string message;
	message.resize(50);
	int returnValue = LoadStringA(NULL, 4, message.data(), message.size());
	if (returnValue == 0)
	{
		std::cout << "Failed to read resource";
		return 1;
	}

	message.resize(returnValue);
	std::cout << message;
	return 0;
}

message.h:

#define MESSAGE_TO_PRINT "Hello from header"

resources.rc:

#include <winresrc.h>

#include "message.h"

STRINGTABLE
BEGIN
4 MESSAGE_TO_PRINT
END

Edit: Not sure if the "MESSAGE_TO_PRINT" in a separate header will work because I haven't implemented the include_directories and raw_headers logic, but it can be just copied into resources.rc.

facebook-github-bot · 2024-03-06T21:20:38Z

@KapJI has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Summary: Adds the ability to compile resource files: https://learn.microsoft.com/en-us/windows/win32/menurc/about-resource-files More details are in the commit messages. X-link: facebook/buck2#581 Reviewed By: JakobDegen Differential Revision: D54603649 Pulled By: KapJI fbshipit-source-id: fcf56a919b8b6245000e71565894dc7c5e1c0c3f

Summary: Like other cxx rules it supports specifying "headers" or "include_directories" and "raw_headers". As a test, I added a header dependency to the test I provided in the previous PR: facebook/buck2#581. The final result being the following files: BUCK: ``` # A rule that includes a single .rc file and compiles it into an object file. windows_resource( name = "resources", srcs = [ "resources.rc", ], header_namespace = "", headers = ["message.h"], ) # A rule that links against the above windows_resource rule. # A rule that includes a single .rc file and compiles it into an object file. windows_resource( name = "resources", srcs = [ "resources.rc", ], header_namespace = "", headers = ["message.h"], ) # A rule that links against the above windows_resource rule. cxx_binary( name = "app", srcs = [ "main.cpp", ], deps = [ ":resources" ], linker_flags = [ "User32.lib", ], ) ``` main.cpp: ``` #include <iostream> #include <Windows.h> int main(int argc, const char* argv[]) { std::string message; message.resize(50); int returnValue = LoadStringA(NULL, 4, message.data(), message.size()); if (returnValue == 0) { std::cout << "Failed to read resource"; return 1; } message.resize(returnValue); std::cout << message; return 0; } ``` message.h: ``` #define MESSAGE_TO_PRINT "Hello from header" ``` resources.rc: ``` #include <winresrc.h> #include "message.h" STRINGTABLE BEGIN 4 MESSAGE_TO_PRINT END ``` X-link: facebook/buck2#607 Reviewed By: JakobDegen Differential Revision: D55633608 Pulled By: KapJI fbshipit-source-id: 6d47acaaa0eb77a653b33d46c8c26088fbf8f0f8

Summary: Like other cxx rules it supports specifying "headers" or "include_directories" and "raw_headers". As a test, I added a header dependency to the test I provided in the previous PR: #581. The final result being the following files: BUCK: ``` # A rule that includes a single .rc file and compiles it into an object file. windows_resource( name = "resources", srcs = [ "resources.rc", ], header_namespace = "", headers = ["message.h"], ) # A rule that links against the above windows_resource rule. # A rule that includes a single .rc file and compiles it into an object file. windows_resource( name = "resources", srcs = [ "resources.rc", ], header_namespace = "", headers = ["message.h"], ) # A rule that links against the above windows_resource rule. cxx_binary( name = "app", srcs = [ "main.cpp", ], deps = [ ":resources" ], linker_flags = [ "User32.lib", ], ) ``` main.cpp: ``` #include <iostream> #include <Windows.h> int main(int argc, const char* argv[]) { std::string message; message.resize(50); int returnValue = LoadStringA(NULL, 4, message.data(), message.size()); if (returnValue == 0) { std::cout << "Failed to read resource"; return 1; } message.resize(returnValue); std::cout << message; return 0; } ``` message.h: ``` #define MESSAGE_TO_PRINT "Hello from header" ``` resources.rc: ``` #include <winresrc.h> #include "message.h" STRINGTABLE BEGIN 4 MESSAGE_TO_PRINT END ``` Pull Request resolved: #607 Reviewed By: JakobDegen Differential Revision: D55633608 Pulled By: KapJI fbshipit-source-id: 6d47acaaa0eb77a653b33d46c8c26088fbf8f0f8

Summary: Adds the ability to compile resource files: https://learn.microsoft.com/en-us/windows/win32/menurc/about-resource-files More details are in the commit messages. X-link: facebook/buck2#581 Reviewed By: JakobDegen Differential Revision: D54603649 Pulled By: KapJI fbshipit-source-id: fcf56a919b8b6245000e71565894dc7c5e1c0c3f

Summary: Like other cxx rules it supports specifying "headers" or "include_directories" and "raw_headers". As a test, I added a header dependency to the test I provided in the previous PR: facebook/buck2#581. The final result being the following files: BUCK: ``` # A rule that includes a single .rc file and compiles it into an object file. windows_resource( name = "resources", srcs = [ "resources.rc", ], header_namespace = "", headers = ["message.h"], ) # A rule that links against the above windows_resource rule. # A rule that includes a single .rc file and compiles it into an object file. windows_resource( name = "resources", srcs = [ "resources.rc", ], header_namespace = "", headers = ["message.h"], ) # A rule that links against the above windows_resource rule. cxx_binary( name = "app", srcs = [ "main.cpp", ], deps = [ ":resources" ], linker_flags = [ "User32.lib", ], ) ``` main.cpp: ``` #include <iostream> #include <Windows.h> int main(int argc, const char* argv[]) { std::string message; message.resize(50); int returnValue = LoadStringA(NULL, 4, message.data(), message.size()); if (returnValue == 0) { std::cout << "Failed to read resource"; return 1; } message.resize(returnValue); std::cout << message; return 0; } ``` message.h: ``` #define MESSAGE_TO_PRINT "Hello from header" ``` resources.rc: ``` #include <winresrc.h> #include "message.h" STRINGTABLE BEGIN 4 MESSAGE_TO_PRINT END ``` X-link: facebook/buck2#607 Reviewed By: JakobDegen Differential Revision: D55633608 Pulled By: KapJI fbshipit-source-id: 6d47acaaa0eb77a653b33d46c8c26088fbf8f0f8

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Feb 26, 2024

Overhatted force-pushed the addWindowsResource branch from 5fb3720 to fa5563f Compare February 27, 2024 20:02

Overhatted added 2 commits February 28, 2024 21:11

Overhatted force-pushed the addWindowsResource branch from fa5563f to 7f890f7 Compare February 28, 2024 21:12

Overhatted marked this pull request as ready for review February 29, 2024 08:43

KapJI reviewed Mar 4, 2024

View reviewed changes

Apply suggestions from code review

df3154a

Co-authored-by: Ruslan Sayfutdinov <ruslan@sayfutdinov.com>

KapJI reviewed Mar 6, 2024

View reviewed changes

prelude/cxx/windows_resource.bzl Outdated Show resolved Hide resolved

Overhatted added 2 commits March 6, 2024 21:03

Using rc and cvtres compiler flags

fb73fa7

Using double instead of single quotes

8f600bf

Overhatted force-pushed the addWindowsResource branch from 929f328 to 8f600bf Compare March 6, 2024 21:08

jrmolin mentioned this pull request Mar 6, 2024

[vswhere.py] Add support for EWDK to the prelude / msvc toolchain #591

Closed

Using a list instead of a tuple in vswhere.py

2fae306

facebook-github-bot closed this in 9dca2c0 Mar 11, 2024

Overhatted mentioned this pull request Mar 17, 2024

Added support for headers in the windows_resource rule #607

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding rule to build Windows .rc files #581

Adding rule to build Windows .rc files #581

Overhatted commented Feb 26, 2024

ndmitchell commented Mar 3, 2024

KapJI left a comment

KapJI Mar 4, 2024

Overhatted Mar 6, 2024

KapJI Mar 6, 2024

Overhatted Mar 7, 2024

KapJI commented Mar 4, 2024 •

edited

Overhatted commented Mar 6, 2024 •

edited

facebook-github-bot commented Mar 6, 2024

Adding rule to build Windows .rc files #581

Adding rule to build Windows .rc files #581

Conversation

Overhatted commented Feb 26, 2024

ndmitchell commented Mar 3, 2024

KapJI left a comment

Choose a reason for hiding this comment

KapJI Mar 4, 2024

Choose a reason for hiding this comment

Overhatted Mar 6, 2024

Choose a reason for hiding this comment

KapJI Mar 6, 2024

Choose a reason for hiding this comment

Overhatted Mar 7, 2024

Choose a reason for hiding this comment

KapJI commented Mar 4, 2024 • edited

Overhatted commented Mar 6, 2024 • edited

facebook-github-bot commented Mar 6, 2024

KapJI commented Mar 4, 2024 •

edited

Overhatted commented Mar 6, 2024 •

edited