Add option for cpp_extensions to compile standalone executable #47862

robieta · 2020-11-12T17:40:04Z

Stack from ghstack:

Enable callgrind collection for C++ snippets #47865 Enable callgrind collection for C++ snippets
Add support for timing C++ snippets. #47864 Add support for timing C++ snippets.
Rework compat bindings. #47863 Rework compat bindings.
Add option for cpp_extensions to compile standalone executable #47862 Add option for cpp_extensions to compile standalone executable
Expose CXX_FLAGS through __config__ #47861 Expose CXX_FLAGS through config

Differential Revision: D25199265

[ghstack-poisoned]

dr-ci · 2020-11-12T19:38:25Z

💊 CI failures summary and remediations

As of commit e778589 (more details on the Dr. CI page):

✅ None of the CI failures appear to be your fault 💚

1/1 broken upstream at merge base 0213a38 on Nov 30 from 7:48am to 3:47pm PDT (15 commits; 5bb2a87 - d74f2d2)

🚧 1 fixed upstream failure:

These were probably caused by upstream breakages that were already fixed.

Please rebase on the viable/strict branch (expand for instructions)

Since your merge base is older than viable/strict, run these commands:

git fetch https://github.com/pytorch/pytorch viable/strict
git rebase FETCH_HEAD

Check out the recency history of this "viable master" tracking branch.

pytorch_bazel_build on Nov 30 from 7:48am to 3:47pm PDT (15 commits; 5bb2a87 - d74f2d2)
- 🔁 rerun

This comment was automatically generated by Dr. CI (expand for details).

Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions on the GitHub issue tracker or post in the (internal) Dr. CI Users group.

See how this bot performed.

This comment has been revised 32 times.

ezyang · 2020-11-13T17:16:21Z

torch/utils/cpp_extension.py

@@ -23,6 +23,13 @@


 IS_WINDOWS = sys.platform == 'win32'
+LIB_EXT = 'pyd' if IS_WINDOWS else 'so'
+EXEC_EXT = 'exe' if IS_WINDOWS else 'o'


exec extension is o on linux? Really?

I wound up just omitting it altogether. (Which I guess is the most UNIX-y thing to do.)

ezyang · 2020-11-13T17:18:15Z

torch/utils/cpp_extension.py

-    if IS_WINDOWS:
+    if is_standalone:
+        ldflags = extra_ldflags
+    elif IS_WINDOWS:


nittiest of nits: it would feel better if it was:

if is_standalone: ... else: if IS_WINDOWS: ... else: ...

as this communicates better that there aren't three cases, there are two cases, and a subdivision in the second case.

Fair point, though my cyclomatic complexity alarm went off for the double nest. I wound up defining SHARED_FLAG since platform is import constant, and then just did ldflags = ([] if is_standalone else [SHARED_FLAG]) + extra_ldflags. A cynic might argue that I'm being too cute / python-y, but hopefully that conveys the intent of "either it's standalone, or slap a --plays_well_with_others stamp on it.)

ezyang · 2020-11-13T17:19:17Z

torch/utils/cpp_extension.py

-def _import_module_from_library(module_name, path, is_python_module):
+def _import_module_from_library(module_name, path, is_python_module, is_standalone):
+    if is_standalone:
+        if IS_WINDOWS and TORCH_LIB_PATH not in os.getenv('PATH', ''):


Technically this is wrong, because if I have f'{TORCH_LIB_PATH}/blah/blah in PATH it will incorrectly detect that the PATH is present when it actually isn't. I also get a bit nervous about forward slash and backward slash confusion here.

Good point. I added .split(';') so now it should actually do the right thing. There shouldn't be any forward-backward slash shenaniganry since I use os.path.blah for everything. (Right?)

It will be strictly safe if you use os.path.samefile, which automatically handles cases and slashes. e.g.

>>> os.path.samefile('C:\\Windows', 'c:/windows') True

But it requires the existence of the paths. So maybe you'll need to use it like os.path.exists(a) and os.path.exists(b) and os.path.samefile(a, b).

torch/utils/cpp_extension.py

ezyang · 2020-11-13T17:22:55Z

torch/utils/cpp_extension.py

+        extra_ldflags.append(f'/LIBPATH:{TORCH_LIB_PATH}')
+        if not is_standalone:
+            extra_ldflags.append('torch_python.lib')
+            extra_ldflags.append(f'/LIBPATH:{python_lib_path}')


By the way, one could conceivable want a standalone executable that still has the Python interpreter. This use case would be when the top level runner is a C++ program, but then calls into the Python runtime. Pretty obscure, but it's a valid configuration. (No action needed.)

ezyang · 2020-11-13T17:23:48Z

test/test_utils.py

+                    stdout=subprocess.PIPE,
+                )
+                self.assertEqual(r.returncode, 0)
+                self.assertEqual(


expect test?

In this case no, because:

expecttest doesn't handle the loop properly. (Tries to double update.)

The replacement is completely dedented.

I tried it (factoring the run bits so there were two literal tests) and it was just ugly and awkward. If there were more test cases or the output was less trivial, yeah I'd convert it to an expect test.

ezyang

just nits

…able" [ghstack-poisoned]

…able" Differential Revision: [D25199265](https://our.internmc.facebook.com/intern/diff/D25199265) [ghstack-poisoned]

facebook-github-bot · 2020-12-02T05:24:17Z

@robieta merged this pull request in 07f038a.

Add option for cpp_extensions to compile standalone executable

cbf28f8

[ghstack-poisoned]

robieta requested review from ezyang, fmassa, goldsborough and soumith as code owners November 12, 2020 17:40

robieta mentioned this pull request Nov 12, 2020

Expose CXX_FLAGS through __config__ #47861

Closed

facebook-github-bot added the cla signed label Nov 12, 2020

This was referenced Nov 12, 2020

Rework compat bindings. #47863

Closed

Add support for timing C++ snippets. #47864

Closed

Enable callgrind collection for C++ snippets #47865

Closed

robieta requested review from malfet and peterjc123 November 12, 2020 17:55

peterjc123 approved these changes Nov 13, 2020

View reviewed changes

ezyang reviewed Nov 13, 2020

View reviewed changes

torch/utils/cpp_extension.py Outdated Show resolved Hide resolved

ezyang reviewed Nov 13, 2020

View reviewed changes

ezyang approved these changes Nov 13, 2020

View reviewed changes

Taylor Robie added 8 commits November 13, 2020 16:38

Update on "Add option for cpp_extensions to compile standalone execut…

0bc71ea

…able" [ghstack-poisoned]

Update on "Add option for cpp_extensions to compile standalone execut…

dc28bc8

…able" [ghstack-poisoned]

Update on "Add option for cpp_extensions to compile standalone execut…

217980d

…able" [ghstack-poisoned]

Update on "Add option for cpp_extensions to compile standalone execut…

a2c6a29

…able" [ghstack-poisoned]

Update on "Add option for cpp_extensions to compile standalone execut…

f821307

…able" Differential Revision: [D25199265](https://our.internmc.facebook.com/intern/diff/D25199265) [ghstack-poisoned]

Update on "Add option for cpp_extensions to compile standalone execut…

9befa9e

…able" Differential Revision: [D25199265](https://our.internmc.facebook.com/intern/diff/D25199265) [ghstack-poisoned]

Update on "Add option for cpp_extensions to compile standalone execut…

9315e17

…able" Differential Revision: [D25199265](https://our.internmc.facebook.com/intern/diff/D25199265) [ghstack-poisoned]

Update on "Add option for cpp_extensions to compile standalone execut…

e778589

…able" Differential Revision: [D25199265](https://our.internmc.facebook.com/intern/diff/D25199265) [ghstack-poisoned]

facebook-github-bot closed this in 07f038a Dec 2, 2020

facebook-github-bot added the Merged label Dec 2, 2020

facebook-github-bot deleted the gh/robieta/2/head branch December 5, 2020 15:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add option for cpp_extensions to compile standalone executable #47862

Add option for cpp_extensions to compile standalone executable #47862

robieta commented Nov 12, 2020 •

edited

dr-ci bot commented Nov 12, 2020 •

edited

ezyang Nov 13, 2020

robieta Nov 14, 2020

ezyang Nov 13, 2020

robieta Nov 14, 2020 •

edited

ezyang Nov 13, 2020

robieta Nov 14, 2020

peterjc123 Nov 14, 2020 •

edited

ezyang Nov 13, 2020

ezyang Nov 13, 2020

robieta Nov 14, 2020

ezyang left a comment

facebook-github-bot commented Dec 2, 2020

Add option for cpp_extensions to compile standalone executable #47862

Add option for cpp_extensions to compile standalone executable #47862

Conversation

robieta commented Nov 12, 2020 • edited

dr-ci bot commented Nov 12, 2020 • edited

💊 CI failures summary and remediations

🚧 1 fixed upstream failure:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robieta Nov 14, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterjc123 Nov 14, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ezyang left a comment

Choose a reason for hiding this comment

facebook-github-bot commented Dec 2, 2020

robieta commented Nov 12, 2020 •

edited

dr-ci bot commented Nov 12, 2020 •

edited

robieta Nov 14, 2020 •

edited

peterjc123 Nov 14, 2020 •

edited