Fix native extension lookup

Fix native extension lookup

This CL allows native extension lookup to defer to the platform's
library lookup (e.g. dlopen) when the library cannot be found next to
the importing Dart library. It also allows dart-ext: to accept an
absolute path to the native library.

R=asiva@google.com

Committed: https://github.com/dart-lang/sdk/commit/b990bc37948b750c4f6985a8575adae4aaee1dee

[zra, 2016-08-29 19:56:46]

zra@google.com changed reviewers:
+ asiva@google.com

[siva, 2016-08-30 01:22:58]

asiva@google.com changed reviewers:
+ whesse@google.com

[siva, 2016-08-30 01:23:00]

+whesse

We need to figure out how to change the native extensions documentation to
reflect this change and communicate it in some form for the 1.20 release

LGTM with some comments.

https://codereview.chromium.org/2285223003/diff/40001/runtime/bin/loader.cc
File runtime/bin/loader.cc (right):

https://codereview.chromium.org/2285223003/diff/40001/runtime/bin/loader.cc#n...
runtime/bin/loader.cc:317: (strstr(extension_path, File::PathSeparator()) !=
NULL))) {
For non windows hosts looks like this check is repeated twice (i.e
strchr(extension_path, '/')

https://codereview.chromium.org/2285223003/diff/40001/runtime/bin/utils.h
File runtime/bin/utils.h (right):

https://codereview.chromium.org/2285223003/diff/40001/runtime/bin/utils.h#new...
runtime/bin/utils.h:88: static char* StrNDup(const char* s, intptr_t n);
Now that you have this implemented here, I am wondering if the function in os.h
should just return Utils::StrNDup(s, n);

[zra, 2016-08-30 02:59:02]

On 2016/08/30 01:23:00, siva wrote:
> We need to figure out how to change the native extensions documentation to
> reflect this change and communicate it in some form for the 1.20 release

This looks like the only documentation:
https://www.dartlang.org/articles/dart-vm/native-extensions.

Currently, it is incorrect, saying, "...the URI specifies a path to the shared
library, either absolute or relative to the directory containing the Dart
library."

It looks like the article source is here:
https://github.com/dart-lang/site-www/blob/master/src/_articles/dart-vm/nativ...

I think I should send a CL to correct the text there, and update the CHANGELOG
in this CL, which I'll do tomorrow.

How does that sound?

[zra, 2016-08-30 16:05:18]

I've updated the change log and started a pull request for the doc change here:

https://github.com/dart-lang/site-www/pull/122

https://codereview.chromium.org/2285223003/diff/40001/runtime/bin/loader.cc
File runtime/bin/loader.cc (right):

https://codereview.chromium.org/2285223003/diff/40001/runtime/bin/loader.cc#n...
runtime/bin/loader.cc:317: (strstr(extension_path, File::PathSeparator()) !=
NULL))) {
On 2016/08/30 01:22:59, siva wrote:
> For non windows hosts looks like this check is repeated twice (i.e
> strchr(extension_path, '/')

Avoided the duplicate test by checking the path separator instead of platform
and pulled the test out to its own function.

https://codereview.chromium.org/2285223003/diff/40001/runtime/bin/utils.h
File runtime/bin/utils.h (right):

https://codereview.chromium.org/2285223003/diff/40001/runtime/bin/utils.h#new...
runtime/bin/utils.h:88: static char* StrNDup(const char* s, intptr_t n);
On 2016/08/30 01:22:59, siva wrote:
> Now that you have this implemented here, I am wondering if the function in
os.h
> should just return Utils::StrNDup(s, n);

Code in runtime/vm shouldn't depend on code in runtime/bin. I'll look into
moving this and other util.h stuff to runtime/platform in another CL.

[zra, 2016-08-30 16:31:51]

Description was changed from

==========
Fix native extension lookup

This CL allows native extension lookup to defer to the platform's
library lookup (e.g. dlopen) when the library cannot be found next to
the importing Dart library. It also allows dart-ext: to accept an
absolute path to the native library.
==========

to

==========
Fix native extension lookup

This CL allows native extension lookup to defer to the platform's
library lookup (e.g. dlopen) when the library cannot be found next to
the importing Dart library. It also allows dart-ext: to accept an
absolute path to the native library.

R=asiva@google.com

Committed:
https://github.com/dart-lang/sdk/commit/b990bc37948b750c4f6985a8575adae4aaee1dee
==========

[zra, 2016-08-30 16:31:52]

Committed patchset #6 (id:100001) manually as
b990bc37948b750c4f6985a8575adae4aaee1dee (presubmit successful).