Skip to content

Wraps all git executions in a Git(...).*, use *=noglob on Windows#172495

Merged
auto-submit[bot] merged 7 commits intoflutter:masterfrom
matanlurey:wrap-git-executions
Jul 23, 2025
Merged

Wraps all git executions in a Git(...).*, use *=noglob on Windows#172495
auto-submit[bot] merged 7 commits intoflutter:masterfrom
matanlurey:wrap-git-executions

Conversation

@matanlurey
Copy link
Contributor

@matanlurey matanlurey commented Jul 21, 2025

Closes #74165.

The original issue called for, on Windows, telling CYGWIN to use =noglob, to work around some git operation errors that happen when using non-native Git. There ... was no great way to do this with the existing codebase without, IMO, adding lots of confusing code.

So, I refactored all the calls of:

  • before: processUtils.<method>(['git', ...args], ...params)
  • after: git.<method>([...args], ...params)

... and implicitly add the new environment variables, if Platform.isWindows.

Did some minor test cleanup and process execution cleanup while I was at it.

@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Jul 21, 2025
@matanlurey matanlurey requested a review from chingjun July 21, 2025 22:21
Copy link
Contributor

@chingjun chingjun 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 minor nits.

await globals.processUtils.run(
<String>['git', 'fetch', '--tags'],
await globals.git.run(
<String>['fetch', '--tags'],
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Remove the <String> to be consistent with the rest of the changes in this file. Same on line 339.

/// Creates a wrapper that executes `git` using [runProcessWith].
Git({
required Platform currentPlatform,
required ProcessUtils runProcessWith, //
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove the //?

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!

required this.flutterRoot,
required this.fs,
}) : _clock = clock,
_gitExecutable = git;
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason to call this _gitExecutable instead of _git like everywhere else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed!

@matanlurey matanlurey added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 21, 2025
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jul 21, 2025
@auto-submit
Copy link
Contributor

auto-submit bot commented Jul 21, 2025

autosubmit label was removed for flutter/flutter/172495, because - The status or check suite Windows tool_tests_commands has failed. Please fix the issues identified (or deflake) before re-applying this label.

@matanlurey matanlurey force-pushed the wrap-git-executions branch from 93a9fed to a5fc080 Compare July 22, 2025 19:59
@matanlurey matanlurey added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 22, 2025
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jul 22, 2025
@auto-submit
Copy link
Contributor

auto-submit bot commented Jul 22, 2025

autosubmit label was removed for flutter/flutter/172495, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

@matanlurey matanlurey added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 23, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Jul 23, 2025
Merged via the queue into flutter:master with commit 6fd30fd Jul 23, 2025
142 of 143 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jul 23, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 23, 2025
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Jul 23, 2025
flutter/flutter@ee0cc66...afba7d7

2025-07-23 [email protected] Remove GtkGLArea and render directly into GtkDrawingArea (flutter/flutter#172343)
2025-07-23 [email protected] Prefix generated Dart plugin imports for `registerWith` (flutter/flutter#172511)
2025-07-23 [email protected] Remove stale references to `.packages` in tool tests (flutter/flutter#172582)
2025-07-23 [email protected] Wraps all `git` executions in a `Git(...).*`, use `*=noglob` on Windows (flutter/flutter#172495)
2025-07-22 [email protected] Omit instruction to `cd .` after `flutter create` (flutter/flutter#172513)
2025-07-22 [email protected] Improve assertion message in `AlignmentDirectional.resolve` (flutter/flutter#172096)
2025-07-22 [email protected] Update warnGradleVersion to `8.7.0` (flutter/flutter#172576)
2025-07-22 [email protected] Use a fake representation of `cache/artifacts/gradle_wrapper` (flutter/flutter#172503)
2025-07-22 [email protected] Revert #160653 Fix view removal process for AutofillContextAction.cancel (flutter/flutter#172490)
2025-07-22 [email protected] Bump meta to 0.17.0 (flutter/flutter#172541)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
azatech pushed a commit to azatech/flutter that referenced this pull request Jul 28, 2025
…ws (flutter#172495)

Closes flutter#74165.

The original issue called for, on Windows, telling `CYGWIN` to use
`=noglob`, to work around some git operation errors that happen when
using non-native Git. There ... was no great way to do this with the
existing codebase without, IMO, adding lots of confusing code.

So, I refactored all the calls of:
- before: `processUtils.<method>(['git', ...args], ...params)` 
- after: `git.<method>([...args], ...params)`

... and implicitly add the new environment variables, if
`Platform.isWindows`.

Did some minor test cleanup and process execution cleanup while I was at
it.
vashworth pushed a commit to vashworth/packages that referenced this pull request Jul 30, 2025
flutter/flutter@ee0cc66...afba7d7

2025-07-23 [email protected] Remove GtkGLArea and render directly into GtkDrawingArea (flutter/flutter#172343)
2025-07-23 [email protected] Prefix generated Dart plugin imports for `registerWith` (flutter/flutter#172511)
2025-07-23 [email protected] Remove stale references to `.packages` in tool tests (flutter/flutter#172582)
2025-07-23 [email protected] Wraps all `git` executions in a `Git(...).*`, use `*=noglob` on Windows (flutter/flutter#172495)
2025-07-22 [email protected] Omit instruction to `cd .` after `flutter create` (flutter/flutter#172513)
2025-07-22 [email protected] Improve assertion message in `AlignmentDirectional.resolve` (flutter/flutter#172096)
2025-07-22 [email protected] Update warnGradleVersion to `8.7.0` (flutter/flutter#172576)
2025-07-22 [email protected] Use a fake representation of `cache/artifacts/gradle_wrapper` (flutter/flutter#172503)
2025-07-22 [email protected] Revert #160653 Fix view removal process for AutofillContextAction.cancel (flutter/flutter#172490)
2025-07-22 [email protected] Bump meta to 0.17.0 (flutter/flutter#172541)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
ksokolovskyi pushed a commit to ksokolovskyi/flutter that referenced this pull request Aug 19, 2025
…ws (flutter#172495)

Closes flutter#74165.

The original issue called for, on Windows, telling `CYGWIN` to use
`=noglob`, to work around some git operation errors that happen when
using non-native Git. There ... was no great way to do this with the
existing codebase without, IMO, adding lots of confusing code.

So, I refactored all the calls of:
- before: `processUtils.<method>(['git', ...args], ...params)` 
- after: `git.<method>([...args], ...params)`

... and implicitly add the new environment variables, if
`Platform.isWindows`.

Did some minor test cleanup and process execution cleanup while I was at
it.
mboetger pushed a commit to mboetger/flutter that referenced this pull request Sep 18, 2025
…ws (flutter#172495)

Closes flutter#74165.

The original issue called for, on Windows, telling `CYGWIN` to use
`=noglob`, to work around some git operation errors that happen when
using non-native Git. There ... was no great way to do this with the
existing codebase without, IMO, adding lots of confusing code.

So, I refactored all the calls of:
- before: `processUtils.<method>(['git', ...args], ...params)` 
- after: `git.<method>([...args], ...params)`

... and implicitly add the new environment variables, if
`Platform.isWindows`.

Did some minor test cleanup and process execution cleanup while I was at
it.
korca0220 pushed a commit to korca0220/flutter that referenced this pull request Sep 22, 2025
…ws (flutter#172495)

Closes flutter#74165.

The original issue called for, on Windows, telling `CYGWIN` to use
`=noglob`, to work around some git operation errors that happen when
using non-native Git. There ... was no great way to do this with the
existing codebase without, IMO, adding lots of confusing code.

So, I refactored all the calls of:
- before: `processUtils.<method>(['git', ...args], ...params)` 
- after: `git.<method>([...args], ...params)`

... and implicitly add the new environment variables, if
`Platform.isWindows`.

Did some minor test cleanup and process execution cleanup while I was at
it.
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 12, 2025
lucaantonelli pushed a commit to lucaantonelli/flutter that referenced this pull request Nov 21, 2025
…ws (flutter#172495)

Closes flutter#74165.

The original issue called for, on Windows, telling `CYGWIN` to use
`=noglob`, to work around some git operation errors that happen when
using non-native Git. There ... was no great way to do this with the
existing codebase without, IMO, adding lots of confusing code.

So, I refactored all the calls of:
- before: `processUtils.<method>(['git', ...args], ...params)` 
- after: `git.<method>([...args], ...params)`

... and implicitly add the new environment variables, if
`Platform.isWindows`.

Did some minor test cleanup and process execution cleanup while I was at
it.
JSUYA added a commit to JSUYA/flutter-tizen that referenced this pull request Dec 10, 2025
JSUYA added a commit to JSUYA/flutter-tizen that referenced this pull request Dec 15, 2025
JSUYA added a commit to JSUYA/flutter-tizen that referenced this pull request Feb 5, 2026
Replace ad-hoc git invocation with globals.git.logSync to match upstream behavior and improve Windows compatibility.\n\nRefs: flutter/flutter#172495\nhttps://github.com/flutter/flutter/pull/172495
JSUYA added a commit to JSUYA/flutter-tizen that referenced this pull request Feb 10, 2026
Replace ad-hoc git invocation with globals.gitc to match upstream behavior and improve Windows compatibility.

related pr : flutter/flutter#172495
JSUYA added a commit to JSUYA/flutter-tizen that referenced this pull request Feb 10, 2026
Replace ad-hoc git invocation with globals.gitc to match upstream behavior and improve Windows compatibility.

related pr : flutter/flutter#172495
JSUYA added a commit to JSUYA/flutter-tizen that referenced this pull request Feb 12, 2026
Replace ad-hoc git invocation with globals.gitc to match upstream behavior and improve Windows compatibility.

related pr : flutter/flutter#172495
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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.

flutter upgrade fails using Cygwin git: "fatal: Needed a single revision"

2 participants