Skip to content

Make flutter and dart scripts invoke their batch file equivalents on Windows - #59789

Merged
gspencergoog merged 2 commits into
flutter:masterfrom
gspencergoog:fix_gitbash
Jun 18, 2020
Merged

gspencergoog merged 2 commits into
flutter:masterfrom
gspencergoog:fix_gitbash

Conversation

@gspencergoog

@gspencergoog gspencergoog commented Jun 18, 2020 •

Copy link
Copy Markdown
Contributor

Description

This makes the flutter and dart scripts invoke their batch file equivalents if running under MINGW (i.e. git-bash) on Windows.

This allows for proper locking, and makes sure that people aren't using two different (and non-mutally-aware) locking systems when running flutter on Windows.

I also fixed a couple of places where we look for MINGW32, which fails under MINGW64. It just looks for MINGW now.

Related Issues

@gspencergoog gspencergoog added the tool Affects the "flutter" command-line tool. See also t: labels. label Jun 18, 2020
Comment thread bin/dart

if [[ $OS =~ MINGW.* ]]; then
# If we're on Windows, invoke the batch script instead, to get proper locking.
exec "${BIN_DIR}/dart.bat" "$@"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do we want to exit after this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The exec command completely substitutes the given command for the current process, so there's no need to exit (it already has exited by invoking the other script).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Also, the final return value is the return value of the other script.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

cool

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I guess that means we should never reach the other switch cases on MINGW anyway?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What other switch cases? I mean, no, it'll never even reach the "fi" because it execs the other process immediately.

@christopherfujino christopherfujino left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@gspencergoog
gspencergoog merged commit 6db2211 into flutter:master Jun 18, 2020
@gspencergoog
gspencergoog deleted the fix_gitbash branch June 18, 2020 20:29
zljj0818 pushed a commit to zljj0818/flutter that referenced this pull request Jun 22, 2020
…Windows (flutter#59789)

This makes the flutter and dart scripts invoke their batch file equivalents if running under MINGW (i.e. git-bash) on Windows.

This allows for proper locking, and makes sure that people aren't using two different (and non-mutally-aware) locking systems when running flutter on Windows.

I also fixed a couple of places where we look for MINGW32, which fails under MINGW64. It just looks for MINGW now.
mingwandroid pushed a commit to mingwandroid/flutter that referenced this pull request Sep 6, 2020
…Windows (flutter#59789)

This makes the flutter and dart scripts invoke their batch file equivalents if running under MINGW (i.e. git-bash) on Windows.

This allows for proper locking, and makes sure that people aren't using two different (and non-mutally-aware) locking systems when running flutter on Windows.

I also fixed a couple of places where we look for MINGW32, which fails under MINGW64. It just looks for MINGW now.
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Jul 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants