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

Pub Transformers running in parallel instead of in stages #23323

Closed
DartBot opened this issue Apr 24, 2015 · 12 comments
Closed

Pub Transformers running in parallel instead of in stages #23323

DartBot opened this issue Apr 24, 2015 · 12 comments
Labels
closed-as-intended Closed as the reported issue is expected behavior

Comments

@DartBot
Copy link

DartBot commented Apr 24, 2015

This issue was originally filed by @kaendfinger


What steps will reproduce the problem?

  1. Create a new package that uses 2 transformers
  2. Run transformers
  3. Notice that transformers run in parallel

What is the expected output? What do you see instead?

with the given transformer definitions:

transformers:

  • transformer_one
  • transformer_two

this should run the first transformer and then the second one.

What version of the product are you using?

Dart VM version: 1.11.0-edge.45391 (Thu Apr 23 17:40:55 2015) on "macos_x64"

On what operating system?

Mac OS X

What browser (if applicable)?

N/A

Please provide any additional information below.

According to https://www.dartlang.org/tools/pub/assets-and-transformers.html

To specify that transformers run in parallel, use [transformer_1, ..., transformer_n]. If order matters, put the transformers on separate lines.

For example, consider three transformers, specified as follows:

transformers:

  • [t1, t2]
  • t3
@sethladd
Copy link
Contributor

Added Area-Pub, Triaged labels.

@nex3
Copy link
Member

nex3 commented Apr 24, 2015

What exactly makes you think they're running in parallel?


Added NeedsInfo label.

@DartBot
Copy link
Author

DartBot commented Apr 24, 2015

This comment was originally written by @kaendfinger


When I added print statements to my 2 transformers, around the work they do, it went like this:

transformer 1 started
transformer 2 started
transformer 1 finished
transformer 2 finished

@nex3
Copy link
Member

nex3 commented Apr 24, 2015

What I should have said was, please provide code that demonstrates this issue.

@DartBot
Copy link
Author

DartBot commented Apr 24, 2015

This comment was originally written by @kaendfinger


Use the following pubspec:

name: pub_transformer_bug
dependencies:
  libffi6_extension: ">=0.0.3 <0.0.4"
  unsafe_extension: ">=0.0.16 <0.0.17"
transformers:

  • unsafe_extension
  • libffi6_extension

Modify .pub-cache/hosted/pub.dartlang.org/libffi6_extension-0.0.3/lib/transformer.dart and .pub-cache/hosted/pub.dartlang.org/unsafe_extension-0.0.16/lib/transformer.dart and add print statements around the following code in each file:

    try {
      var path = _resolvePackagePath(filepath);
      FileUtils.chdir(path);
      var installer = new Installer();
      await installer.install([]);
    } finally {
      FileUtils.chdir(_workingDirectory);
    }

run pub get

@nex3
Copy link
Member

nex3 commented Apr 24, 2015

Changing the working directory is never going to work out well for you in transformers. It's 100% intended for transformers to run in parallel on multiple files, so you're going to have serious race conditions any time you have multiple transformers modifying the same global state, including the working directory.

@DartBot
Copy link
Author

DartBot commented Apr 24, 2015

This comment was originally written by @kaendfinger


Ahh I didn't even pay attention to the fact that it's doing that. So that would be the issue I guess then, correct? If so, go ahead in close this. The reason why we change directories for these transformers is really just a hack (acts like an install hook for us).

Thanks for checking into it though!

@nex3
Copy link
Member

nex3 commented Apr 24, 2015

Added AsDesigned label.

@DartBot
Copy link
Author

DartBot commented Apr 25, 2015

This comment was originally written by @mezoni


It's 100% intended for transformers to run in parallel on multiple files, so you're going to have serious race conditions any time you have multiple transformers modifying the same global state, including the working directory.

Need special kind of the transformers which can be sure that their instance run exclusively.

Eg.

abstract class PreinstallTransformer implements Transformer {
  Future apply(InstallationTransform transform);
}

abstract class PostinstallTransformer implements Transformer {
  Future apply(InstallationTransform transform);
}

Where "InstallationTransform" should provide the path to the package in pub-cache.

@DartBot
Copy link
Author

DartBot commented Apr 25, 2015

This comment was originally written by @mezoni


Better way:

abstract class GlobalTransformer implements Transformer {
  String get acceptedPhases;

  Future apply(GlobalTransform transform);
}

Eg.

class MyInstallTransformer extends GlobalTransformer {
  String get acceptedPhases = "preinstall";

  Future apply(GlobalTransform transform) {
    switch(transform.phase) {
      case "preinstall":
        preinstall(transform.packageRoot);
        break;
    }
  }
}

@DartBot
Copy link
Author

DartBot commented Apr 25, 2015

This comment was originally written by @mezoni


I am more than sure that the developers never did anything for the minority (read: for the community). Only for the majority.

And as all we know that the majority is a developers themselves, and the minority is an all rest of the people in the world.

@DartBot
Copy link
Author

DartBot commented Jun 4, 2015

This issue has been moved to dart-lang/pub#13.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-as-intended Closed as the reported issue is expected behavior
Projects
None yet
Development

No branches or pull requests

4 participants