Skip to content

Always precompile scripts before running them #3074

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

Merged
merged 2 commits into from
Aug 23, 2021
Merged

Conversation

jakemac53
Copy link
Contributor

@jakemac53 jakemac53 commented Aug 10, 2021

Closes #3073

The idea is that since we have incremental compilation, its worth actually compiling the scripts and running that compiled script. For mutable packages we still always re-compile before each run, but it will be much faster to do so, as each build will be incremental.

This makes running pub executables from source much faster, and it also means you will get some more immediate feedback (the Building package executable... message). Previously it was just a blank screen for several seconds (depending on size of the app).

Note that this does mean we will always emit some log messages about building scripts for each run. This could break some scripts that rely on the stdout of pub executables. This was always the case for the first run of an immutable package ran with pub run, but would come up more often now. We could consider moving that output to stderr, or suppressing it, but I do think it is actually nice, when running interactively.

@google-cla google-cla bot added the cla: yes label Aug 10, 2021
@jakemac53 jakemac53 requested review from jonasfj and sigurdm August 10, 2021 20:30
Copy link
Contributor

@sigurdm sigurdm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
Nice! - this is awesome!

@sigurdm
Copy link
Contributor

sigurdm commented Aug 12, 2021

Note that this does mean we will always emit some log messages about building scripts for each run. This could break some scripts that rely on the stdout of pub executables.

I'm sure you're aware, but we do have the warningsOnlyUnlessTerminal guard that prevents the output if run non-interactively.

Question is if the information is relevant to the user - maybe we should only show anything if the wait is longer than ~2 seconds...

@jakemac53
Copy link
Contributor Author

I'm sure you're aware, but we do have the warningsOnlyUnlessTerminal guard that prevents the output if run non-interactively.

Oh! I was not aware this was a thing :).

Question is if the information is relevant to the user - maybe we should only show anything if the wait is longer than ~2 seconds...

I don't have any strong opinions here, other than we should probably be consistent between the modes :). Today we always show this message when pre-compiling.

@jakemac53
Copy link
Contributor Author

Fwiw this is ready to merge from my perspective - not sure if you also wanted @jonasfj to take a look? Or if you wanted to iterate on the UX (namely the build message?).

Copy link
Member

@jonasfj jonasfj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r+

@sigurdm sigurdm merged commit 77702ab into master Aug 23, 2021
@jakemac53 jakemac53 deleted the always-precompile branch August 25, 2021 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use incremental compilation when running from source/git
3 participants