Sitelet https://github.com/flutter/flutter/pull/106861
Skip to content

[Windows] Remove the usage of SETLOCAL ENABLEDELAYEDEXPANSION from bat scripts. - #106861

Merged
auto-submit[bot] merged 2 commits into
masterfrom
moko256_win_avoid_enabledelayedexpansion
Jul 14, 2022
Merged

auto-submit[bot] merged 2 commits into
masterfrom
moko256_win_avoid_enabledelayedexpansion

Conversation

@moko256

@moko256 moko256 commented Jun 30, 2022

Copy link
Copy Markdown
Contributor

On Windows, calling another script without CALL from bat script won't back to caller when callee exited. The callee can use last exit code. When the callee exits with exit code, the caller's caller can receive the code.

This PR replaces the usage of SETLOCAL ENABLEDELAYEDEXPANSION with this behavior.
This PR also replaces the usage of it which can be replaced by normal %~%.

Fixes #26571.

No changes in the flutter/tests repo.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@flutter-dashboard

Copy link
Copy Markdown

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@moko256

moko256 commented Jun 30, 2022

Copy link
Copy Markdown
Contributor Author

I added a test.

@goderbauer goderbauer added the tool Affects the "flutter" command-line tool. See also t: labels. label Jul 1, 2022
@christopherfujino

Copy link
Copy Markdown
Contributor

@loic-sharma would you by any chance know how batch files work, and be able to review this change?

@loic-sharma loic-sharma left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@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.

RSLGTM, thanks for this change!

@christopherfujino christopherfujino added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 14, 2022
@auto-submit
auto-submit Bot merged commit 45e212e into master Jul 14, 2022
@auto-submit
auto-submit Bot deleted the moko256_win_avoid_enabledelayedexpansion branch July 14, 2022 21:56
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Jul 15, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 15, 2022
camsim99 pushed a commit to camsim99/flutter that referenced this pull request Aug 10, 2022
pull Bot pushed a commit to TheRakeshPurohit/flutter that referenced this pull request Aug 22, 2026
… and batch entrypoints (flutter#191438)

## Description

Fixes flutter#84270

The root cause of flutter#84270 (delayed variable expansion in batch scripts
stripping `!` from CLI arguments like `^(?!Golden).+`) was previously
addressed in flutter#106861, which removed `SETLOCAL ENABLEDELAYEDEXPANSION`
from `bin/flutter.bat`, `bin/dart.bat`, and `bin/internal/shared.bat`.

However, flutter#84270 was left open without dedicated regression tests. This
PR:
1. Adds a hermetic unit test in `test_test.dart` verifying that
`TestCommand` preserves and forwards negative lookahead regexes
(`^(?!Golden).+`) to `FlutterTestRunner`.
2. Adds a case-insensitive static test in `script_test.dart` asserting
that Windows entrypoint batch scripts do not enable delayed expansion.
3. Unskips and modernizes the integration test in
`variable_expansion_windows_test.dart` verifying that `bin/dart.bat`
preserves lookahead regexes on Windows.

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [ ] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] All existing and new tests are passing.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows failure if install directory has a "!" in it

4 participants