-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Do not use project in do last #139325
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
Do not use project in do last #139325
Conversation
|
Replacement copy works and existing test verifying read only behavior works. I need to add a new test to cover the branching behavior of gradle 8.3 and ensure it does not regress. Thanks to stuart I know I do not need to copy module_test but instead can follow a pattern similar to dev/devicelab/bin/tasks/plugin_test.dart where there a set of tasks with parameters that runs and allows the test to make changes based on the inputs. |
…es the gradle contract
| final String platformLineSep = Platform.isWindows ? '\r\n' : '\n'; | ||
|
|
||
| /// Combines several TaskFunctions with trivial success value into one. | ||
| TaskFunction combine(List<TaskFunction> tasks) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm guessing in follow up PRs you will add more tasks here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes once I can make them pass. Pr fatigue is setting in and I want to get this improvement landed to minimize conflicts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SGTM
| zipStoreBase=GRADLE_USER_HOME | ||
| zipStorePath=wrapper/dists | ||
| distributionUrl=https\://services.gradle.org/distributions/gradle-7.6.3-all.zip | ||
| distributionUrl=https\://services.gradle.org/distributions/gradle-REPLACEME-all.zip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was REPLACEME a note to self to change? or are we interpolating this somehow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is the string we look for when trying to find what to replace.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SGTM
| } else { | ||
| // See https://docs.gradle.org/8.2/dsl/org.gradle.api.tasks.Copy.html#org.gradle.api.tasks.Copy:fileMode | ||
| // See https://github.com/flutter/flutter/pull/50047 | ||
| fileMode 0644 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this works from windows?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe so but dont have a windows machine to check. The test passes on windows, this is the gradle api for setting file mode which works on windows. There is not a windows specific api.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
acknowledged. my understanding is windows' concept of permissions is much less granular than unix and thus there must be some primitive mapping of the octal codes to windows land inside gradle...
...but if the test passes that seems good enough :)
christopherfujino
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
bartekpacia
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but... wow, these are some wild problems haha
flutter/flutter@11a9cb7...a51e33a 2023-12-14 [email protected] Roll Flutter Engine from 223f4b2465dd to caf33276468b (1 revision) (flutter/flutter#140156) 2023-12-14 [email protected] Roll Packages from b5958e2 to 1151191 (10 revisions) (flutter/flutter#140154) 2023-12-14 [email protected] Roll Flutter Engine from a3f9393f9591 to 223f4b2465dd (1 revision) (flutter/flutter#140151) 2023-12-14 [email protected] Roll Flutter Engine from 913446eca57c to a3f9393f9591 (2 revisions) (flutter/flutter#140144) 2023-12-14 [email protected] Roll Flutter Engine from 923f9e29d4b5 to 913446eca57c (1 revision) (flutter/flutter#140132) 2023-12-14 [email protected] Roll Flutter Engine from 9f7004e3e30e to 923f9e29d4b5 (7 revisions) (flutter/flutter#140130) 2023-12-14 [email protected] Add self back to CODEOWNERS (flutter/flutter#140080) 2023-12-14 [email protected] Adapt wording for required Android SDK for plugins (flutter/flutter#140043) 2023-12-14 [email protected] [reland] Support conditional bundling of assets based on `--flavor` (flutter/flutter#139834) 2023-12-14 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll Flutter Engine from 9f7004e3e30e to 45b95f264d63 (1 revision)" (flutter/flutter#140123) 2023-12-14 [email protected] [deps] update Android SDK to 34 (flutter/flutter#138183) 2023-12-14 [email protected] Roll Flutter Engine from 9f7004e3e30e to 45b95f264d63 (1 revision) (flutter/flutter#140108) 2023-12-14 [email protected] Catch `Stopwatch` with static analysis (flutter/flutter#140019) 2023-12-14 [email protected] Overlay supports unconstrained environments (flutter/flutter#139513) 2023-12-14 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 2.22.10 to 3.22.11 (flutter/flutter#140087) 2023-12-14 [email protected] Remove deprecated `ThemeData.selectedRowColor` (flutter/flutter#139080) 2023-12-14 [email protected] Unpin mac_toolchain version (flutter/flutter#139938) 2023-12-14 [email protected] Optimize file transfer when using proxied devices. (flutter/flutter#139968) 2023-12-14 [email protected] Add commonly used parameter names (flutter/flutter#140027) 2023-12-13 [email protected] Roll Flutter Engine from fc3267724e1b to 9f7004e3e30e (4 revisions) (flutter/flutter#140090) 2023-12-13 [email protected] [Windows] Remove header guard from generated key map (flutter/flutter#140082) 2023-12-13 [email protected] Do not use project in do last (flutter/flutter#139325) 2023-12-13 [email protected] Roll pub packages (flutter/flutter#140024) 2023-12-13 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Warn when Gradle plugins are applied using the legacy "apply script method" way" (flutter/flutter#140102) 2023-12-13 [email protected] Swap iOS back button icon in Material app bar (flutter/flutter#134754) 2023-12-13 [email protected] Roll Packages from 80aa46a to b5958e2 (10 revisions) (flutter/flutter#140069) 2023-12-13 [email protected] Roll Flutter Engine from 9039ac78cf03 to fc3267724e1b (26 revisions) (flutter/flutter#140084) 2023-12-13 [email protected] Warn when Gradle plugins are applied using the legacy "apply script method" way (flutter/flutter#139690) 2023-12-13 [email protected] Add self as bundler dependabot reviewer (flutter/flutter#140081) 2023-12-13 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Make tests more resilient to Skia gold failures and refactor flutter_goldens for extensive technical debt removal" (flutter/flutter#140085) 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],[email protected],[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
|
There seems to be an issue when using flutter beta (1a9a60d) And got this error: |
|
I think that's because |
- handle number format exceptions and strip rc information from version compare - add test that handles rc format part 2/n #138523 Helpfully pointed out by [asaarnak](https://github.com/asaarnak) #139325 (comment)
part 1/n for /issues/138523
This PR does not test the 8.3 branch but instead builds the ability to write a branching gradle test.
After fixing this project.exec action there I discovered a second that needs to be removed or migrated in order for the build to work.
Related reading https://docs.gradle.org/8.0/userguide/configuration_cache.html#config_cache:requirements
https://docs.gradle.org/current/javadoc/org/gradle/process/ExecOperations.html (not used but considered)
https://docs.gradle.org/8.2/dsl/org.gradle.api.tasks.Copy.html#org.gradle.api.tasks.Copy:fileMode
Pre-launch Checklist
///).