Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

fix gclient runhooks for windows #20014

Merged
merged 5 commits into from
Jul 24, 2020
Merged

Conversation

jonahwilliams
Copy link
Contributor

@jonahwilliams jonahwilliams commented Jul 24, 2020

Description

pub is pub.bat on windows

@auto-assign auto-assign bot requested a review from flar July 24, 2020 19:56
subprocess.call(["src/third_party/dart/tools/sdks/dart-sdk/bin/pub", "global", "run", "generate_package_config:generate_from_legacy", "src/flutter/tools/const_finder/.packages"])
pub = "src/third_party/dart/tools/sdks/dart-sdk/bin/pub"
if os.name == "nt":
pub = "src/third_party/dart/tools/sdks/dart-sdk/bin/pub.bat"
Copy link
Member

Choose a reason for hiding this comment

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

Actually I just tried this on my local Windows box and it only seems to work with backslashes, like:
subprocess.call(["bin\\cache\\dart-sdk\\bin\\pub.bat"])

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was working for several weeks when I was using dart pub on windows

Copy link
Contributor Author

Choose a reason for hiding this comment

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

switched to backslashes

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: use os.path.join

Copy link
Contributor

Choose a reason for hiding this comment

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

And then you can just conditionally define a string called BAT based on whether it's nt and append that to the output of path.join.

pub = "src/third_party/dart/tools/sdks/dart-sdk/bin/pub"
if os.name == "nt":
pub = "src/third_party/dart/tools/sdks/dart-sdk/bin/pub.bat"
subprocess.call([pub, "global", "activate", "-spath", "./src/flutter/tools/generate_package_config"])
Copy link
Contributor

Choose a reason for hiding this comment

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

use subprocess.check_call

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

@dnfield dnfield left a comment

Choose a reason for hiding this comment

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

LGTM with nit

@jonahwilliams
Copy link
Contributor Author

runhooks passed on windows, landing

@jonahwilliams jonahwilliams merged commit 10adbbd into flutter:master Jul 24, 2020
@jonahwilliams jonahwilliams deleted the windows-bs branch July 24, 2020 20:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants