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

fix: Run Process.runSync in another shell to get Dart version #300

Merged
merged 2 commits into from
May 28, 2022

Conversation

lulupointu
Copy link
Contributor

Description

When trying to get the dart version, dart --version should be run inside a separate shell to avoid failing when running in a sub process.

The failing of this command is actually used here:

bool isPubSubcommand({required MelosWorkspace workspace}) {
try {
return Process.runSync(workspace.sdkTool('pub'), ['--version']).exitCode !=
0;
} on ProcessException {
return true;
}
}

This bug was introduced in 740050c since it changed how to dart version is retrieved.

Type of Change

  • feat -- New feature (non-breaking change which adds functionality)
  • 🛠️ fix -- Bug fix (non-breaking change which fixes an issue)
  • ! -- Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 refactor -- Code refactor
  • ci -- Build configuration change
  • 📝 docs -- Documentation
  • 🗑️ chore -- Chore

@CLAassistant
Copy link

CLAassistant commented May 19, 2022

CLA assistant check
All committers have signed the CLA.

@lulupointu lulupointu changed the title Fix: Run Process.runSync in another shell to get Dart version fix: Run Process.runSync in another shell to get Dart version May 19, 2022
@blaugold
Copy link
Collaborator

blaugold commented May 19, 2022

Thanks for the PR! I don't quite understand yet what the behavior is that this fixes. Do you get an error and if so, what is it?

Edit: #301 has more details on the root cause.

@Salakar Salakar merged commit 0aa81a7 into invertase:main May 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fix: system command call error when trying to run Melos through FVM and using --sdk-path
4 participants