Deprecate `--bind` in favor of just `-lembind`. NFC #16087

sbc100 · 2022-01-21T23:07:55Z

All that the --bind flag does is enable the embind library (native
and JS portions), so I think its more clear if we just use the normal
-l method of including it.

Historically --bind has sounded to some (including myself) like a
verb rather than the name of a library leading to folks to believe that
the linker is actually doing the binding, whereas all the binding is
done either at compile time or at load time. No "binding" happens
during linking.

RReverser · 2022-01-21T23:43:49Z

I'm in favour of this change from semantic perspective. But, just to be clear, that's all it is - a semantic change - and the new syntax doesn't provide any capabilities towards #14729?

sbc100 · 2022-01-21T23:53:21Z

I'm in favour of this change from semantic perspective. But, just to be clear, that's all it is - a semantic change - and the new syntax doesn't provide any capabilities towards #14729?

No it doesn't. Its just a minor step.

RReverser · 2022-01-22T00:13:13Z

Sounds good!

sbc100 · 2022-01-22T01:23:01Z

adding @kripken to make sure he is on board with my plan. To be clear, I'm not determined to remove the old flag.. at least not for a while :)

kripken · 2022-01-25T00:25:06Z

ChangeLog.md

@@ -24,6 +24,9 @@ See docs/process.md for more on how version tagging works.
  `emcc` now uses this mode when the `--embed-file` option is used.  If you
  use `file_packager` directly it is recommended that you switch to the new mode
  by adding `--obj-output` to the command line. (#16050)
+- The `--bind` flag used to enable embind has been deprecated in favor of
+  `-lembind`.  The semantics have not changed and the old flag continues to
+  work. (#16087)


this is no longer in the right place

It looks correct to me.. but perhaps this relates to an earlier version?

Oh, I think I misread it. lgtm

emcc.py

This was originally introduced 10 years ago to work around a bug that occurred on windows vista (See 4ab1c8a). Its not clear if the bug was in cmake or in windows itself, or in the version of python. Windows Vista has been unsupported for a very long time now so I think ever attempted to verify this bug still exists would be very tricky. What is more, the underlying tools such as clang don't support this behavior so any user who suffered from this bug likely wouldn't be able ot use clang outside of emscripten. For example: ``` clang ' --version' clang: error: no such file or directory: ' --version'; did you mean '--version'? clang: error: no input files ``` I don't know of any tools that don't work like that above, so I don't see why we should be any different. The motivation for removing this now is that it came up in discussion when working on code nearby (#16087) and it seems to poinless at best and confusing/incomaptiable at best (since it prevents the use of filenames that start or end in space).

This was originally introduced 10 years ago to work around a bug that occurred on windows vista (See 4ab1c8a). Its not clear if the bug was in cmake or in windows itself, or in the version of python. Windows Vista has been unsupported for a very long time now so I think ever attempted to verify this bug still exists would be very tricky. What is more, the underlying tools such as clang don't support this behavior so any user who suffered from this bug likely wouldn't be able ot use clang outside of emscripten. For example: ``` clang ' --version' clang: error: no such file or directory: ' --version'; did you mean '--version'? clang: error: no input files ``` I don't know of any tools that don't work like that above, so I don't see why we should be any different. The motivation for removing this now is that it came up in discussion when working on code nearby (#16087) and it seems to pointless at best and is also a compatibility issue (since it prevents the use of filenames that start or end in space).

…6115) This was originally introduced 10 years ago to work around a bug that occurred on windows vista (See 4ab1c8a). Its not clear if the bug was in cmake or in windows itself, or in the version of python. Windows Vista has been unsupported for a very long time now so I think even attempted to verify this bug still exists would be very tricky. What is more, the underlying tools such as clang don't support this behavior so any user who suffered from this bug likely wouldn't be able ot use clang outside of emscripten. For example: ``` clang ' --version' clang: error: no such file or directory: ' --version'; did you mean '--version'? clang: error: no input files ``` I don't know of any tools that don't work like that above, so I don't see why we should be any different. The motivation for removing this now is that it came up in discussion when working on code nearby (#16087) and it seems to pointless at best and is also a compatibility issue (since it prevents the use of filenames that start or end in space).

All that the `--bind` flag does is enable the embind library (native and JS portions), so I think its more clear if we just use the normal `-l` method of including it. Historically `--bind` has sounded to some (including myself) like a verb rather than the name of a library leading to folks to believe that the linker is actually doing the binding, whereas all the binding is done either at compile time or at load time. No "binding" happens during linking.

dschuff · 2022-01-28T00:17:49Z

Out of curiosity, can the semantics of embind actually be reduced to just linking a library? How much emcc magic is still needed now and/or if we cared enough in the future?

sbc100 · 2022-01-28T00:23:43Z

As far as I know embind has been nothing more than a library (composed of a JS library and native library). I don't think anything in particular happens at link time (the JS libraries are not doing anything special AFAICT) .. that magic mostly happens in static ctors (which register types) and at runtime dynamically.

If we ever do something more complex such as a wasm-bindgen type tool I think I would advocate for making is a separate executable.

dschuff · 2022-01-28T00:26:26Z

ok cool that makes sense. The js-library-plus-native-library bit is interesting. Is that special-cased for embind, or is it possible for anyone to make libraries that have both JS and wasm components (i.e. that are considered part of the same single library)?

sbc100 · 2022-01-28T00:29:36Z

ok cool that makes sense. The js-library-plus-native-library bit is interesting. Is that special-cased for embind, or is it possible for anyone to make libraries that have both JS and wasm components (i.e. that are considered part of the same single library)?

As of today, a non-core library would need to require something like -lfoo --library-js foo.js to get both components. Core libraries have some magic to make it implicit:

emscripten/tools/building.py

Lines 1396 to 1400 in e35d017

    
           # And some are hybrid and require JS and native libraries to be included 
        
           native_library_map = { 
        
             'embind': 'libembind', 
        
             'GL': 'libGL', 
        
           }

Core JS libraries can also be included via -lfoo.js and I think we should certainly extent that part to user JS libraries.

RReverser · 2022-01-28T03:10:11Z

Core JS libraries can also be included via -lfoo.js and I think we should certainly extent that part to user JS libraries.

FWIW I'm in favour but last time this was attempted, there were some concerns (admittedly, that was ages ago): #4816

sbc100 · 2022-01-28T03:13:47Z

Core JS libraries can also be included via -lfoo.js and I think we should certainly extent that part to user JS libraries.

FWIW I'm in favour but last time this was attempted, there were some concerns (admittedly, that was ages ago): #4816

On nice.. thanks for digging that up. I'll be sure to take that into account.

sbc100 force-pushed the embind_flag branch 2 times, most recently from f05491d to e0b29bf Compare January 21, 2022 23:13

sbc100 requested a review from RReverser January 21, 2022 23:14

sbc100 force-pushed the embind_flag branch from e0b29bf to bc2f8fa Compare January 21, 2022 23:40

RReverser approved these changes Jan 22, 2022

View reviewed changes

sbc100 requested a review from kripken January 22, 2022 01:22

sbc100 force-pushed the embind_flag branch from bc2f8fa to 7281aa0 Compare January 22, 2022 01:24

kripken approved these changes Jan 25, 2022

View reviewed changes

RReverser reviewed Jan 26, 2022

View reviewed changes

emcc.py Show resolved Hide resolved

sbc100 mentioned this pull request Jan 26, 2022

Remove stripping of individual command line arguments in emcc.py. #16115

Merged

sbc100 force-pushed the embind_flag branch from 7281aa0 to 36b1791 Compare January 26, 2022 21:54

sbc100 enabled auto-merge (squash) January 26, 2022 22:36

sbc100 merged commit 96af70d into main Jan 26, 2022

sbc100 deleted the embind_flag branch January 26, 2022 23:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deprecate `--bind` in favor of just `-lembind`. NFC #16087

Deprecate `--bind` in favor of just `-lembind`. NFC #16087

sbc100 commented Jan 21, 2022

RReverser commented Jan 21, 2022

sbc100 commented Jan 21, 2022

RReverser commented Jan 22, 2022

sbc100 commented Jan 22, 2022 •

edited

Loading

kripken Jan 25, 2022

sbc100 Jan 26, 2022

kripken Jan 26, 2022

dschuff commented Jan 28, 2022

sbc100 commented Jan 28, 2022

dschuff commented Jan 28, 2022 •

edited

Loading

sbc100 commented Jan 28, 2022

RReverser commented Jan 28, 2022

sbc100 commented Jan 28, 2022

Deprecate --bind in favor of just -lembind. NFC #16087

Deprecate --bind in favor of just -lembind. NFC #16087

Conversation

sbc100 commented Jan 21, 2022

RReverser commented Jan 21, 2022

sbc100 commented Jan 21, 2022

RReverser commented Jan 22, 2022

sbc100 commented Jan 22, 2022 • edited Loading

kripken Jan 25, 2022

Choose a reason for hiding this comment

sbc100 Jan 26, 2022

Choose a reason for hiding this comment

kripken Jan 26, 2022

Choose a reason for hiding this comment

dschuff commented Jan 28, 2022

sbc100 commented Jan 28, 2022

dschuff commented Jan 28, 2022 • edited Loading

sbc100 commented Jan 28, 2022

RReverser commented Jan 28, 2022

sbc100 commented Jan 28, 2022

Deprecate `--bind` in favor of just `-lembind`. NFC #16087

Deprecate `--bind` in favor of just `-lembind`. NFC #16087

sbc100 commented Jan 22, 2022 •

edited

Loading

dschuff commented Jan 28, 2022 •

edited

Loading