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

dart:isolate spawnUri should have a customizable package root #12474

Closed
nex3 opened this issue Aug 14, 2013 · 19 comments
Closed

dart:isolate spawnUri should have a customizable package root #12474

nex3 opened this issue Aug 14, 2013 · 19 comments
Assignees
Labels
P1 A high priority bug; for example, a single project is unusable or has many test failures
Milestone

Comments

@nex3
Copy link
Member

nex3 commented Aug 14, 2013

Right now, when starting an isolate with spawnUri, the package root is set to the same thing as the parent isolate's package root. This should be customizable; otherwise it's nearly impossible to run code with different dependencies than the host process in an isolate.

@floitschG
Copy link
Contributor

package_isolate_test is affected by this, too.

@nex3
Copy link
Member Author

nex3 commented Feb 27, 2014

Marked this as blocking #17156.

@lrhn
Copy link
Member

lrhn commented Feb 27, 2014

We will need this feature.

It's impossible to do this fully dynamically in dart2js code, since package root is a compile time concept. Does dart2js have a strategy for supporting it?
Should the other main library be compiled separately, and should there be a check that the dynamic package root matches the one passed at compile time, and if it doesn't, the code won't work (just as if the main library URI doesn't match a compiled JS file).


cc @a-siva.
cc @floitschG.

@munificent
Copy link
Member

It's impossible to do this fully dynamically in dart2js code, since package root is a compile time concept. Does dart2js have a strategy for supporting it?

For the record, our use case with pub doesn't require it to work in dart2js so not supporting it on dart2js doesn't block us.

@floitschG
Copy link
Contributor

I think that the other main library should be compiled separately.
Not sure about package-root check.

@nex3
Copy link
Member Author

nex3 commented Sep 9, 2014

Marked this as blocking #20859.

@munificent
Copy link
Member

Adding a milestone since Ivan agreed this would be done by end of 9/12. If I have that wrong, let me know.


Added this to the 1.7 milestone.
Removed Priority-Unassigned label.
Added Priority-High label.

@lrhn
Copy link
Member

lrhn commented Sep 10, 2014

cc @Skabet.

@andersjohnsen
Copy link

The CL is already available here: https://codereview.chromium.org/545483002

Please provide us with feedback as soon as possible (landing this will take a few more days, as we need to coordinate with Dartium efforts).


Set owner to @Skabet.
Added Started label.

@nex3
Copy link
Member Author

nex3 commented Sep 10, 2014

I've patched it in locally and it seems to work fine. It provided a 420ms improvement in one of my test cases, which is great.

@munificent
Copy link
Member

It provided a 420ms improvement in one of my test cases, which is great.

\o/ \o/ \o/

@DartBot
Copy link

DartBot commented Sep 11, 2014

This comment was originally written by @zoechi


When I launch several isolates they can't have different current working directories (in addition to different package roots)? As far as I know this is bound to the process.

When I launch an isolate from code that accesses files using relative paths this doesn't work without setting CWD first, but when I launch more than one isolate with different CWD requirements concurrently these will collide.

I went back from isolates to processes because of this in some use cases.
You might run into this as well - or maybe you have an idea how to solve this with isolates.

@andersjohnsen
Copy link

We are aware of the limitation in regard to CWD and isolates. Can you file a seperate bug for this, so we can track the progress? It's an area we have yet to fully investigate.

@DartBot
Copy link

DartBot commented Sep 11, 2014

This comment was originally written by @zoechi


Done dartbug.com/20903

@andersjohnsen
Copy link

Landed the VM (and dummy Dartium) change today. Reassigning to Dartium.


Removed the owner.
Removed this from the 1.7 milestone.
Removed Area-Library, Library-Isolate labels.
Added Area-Dartium, Triaged labels.

@munificent
Copy link
Member

\o/

@nex3
Copy link
Member Author

nex3 commented Sep 25, 2014

Unmarked this as blocking #20859.

@kasperl
Copy link

kasperl commented Nov 28, 2014

Added this to the 1.9 milestone.

@kevmoo
Copy link
Member

kevmoo commented Feb 18, 2015

Is this fixed?


Set owner to @sgjesse.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 A high priority bug; for example, a single project is unusable or has many test failures
Projects
None yet
Development

No branches or pull requests

9 participants