Skip to content
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

dart sdk roll breaks cocoon via transitive dep uuid_enhanced #82072

Closed
christopherfujino opened this issue May 7, 2021 · 11 comments
Closed

dart sdk roll breaks cocoon via transitive dep uuid_enhanced #82072

christopherfujino opened this issue May 7, 2021 · 11 comments
Assignees
Labels
dependency: dart Dart team may need to help us P2 Important issues not at the top of the work list team Infra upgrades, team productivity, code health, technical debt. See also team: labels. team-infra Owned by Infrastructure team

Comments

@christopherfujino
Copy link
Member

christopherfujino commented May 7, 2021

The validate_ci_yaml test broke on the engine roll #82070 because a transitive dependency (uuid_enhanced, depended on by graphql) extended an UnmodifiableUint8ListView, which is no longer permitted by https://dart.googlesource.com/sdk.git/+/3e9cdc56440bded76d8078a2567910236ad6f7b9%5E%21/#F1

https://ci.chromium.org/p/flutter/builders/try/Linux%20validate_ci_config/1769?

../../../x/w/.pub-cache/hosted/pub.dartlang.org/uuid_enhanced-3.0.2/lib/uuid.dart:9:7: Error: 'UnmodifiableUint8ListView' is restricted and can't be extended or implemented.
class Uuid extends UnmodifiableUint8ListView {
      ^

To clarify, the validate_ci_yaml script does not require graphql; however, the cocoon service itself DOES, so landing this change would probably would lead to cocoon being unable to build with the latest Flutter. I am not sure why customer_testing did not fail.

@christopherfujino christopherfujino added the team-infra Owned by Infrastructure team label May 7, 2021
@christopherfujino christopherfujino self-assigned this May 7, 2021
godofredoc added a commit to godofredoc/flutter that referenced this issue May 7, 2021
The validation is complaining about a dependency. We are disabling it to
allow the engine rolls to continue.

Bug: flutter#82072
@christopherfujino
Copy link
Member Author

Cocoon depends on package uuid_enhanced transitively through package https://pub.dev/packages/graphql

@christopherfujino
Copy link
Member Author

I'm not sure how customer_testing passed on the engine roll, when I run the unit tests of the pinned cocoon version in the test registry with the dart sdk from the engine roll, the unit tests fail.

@christopherfujino
Copy link
Member Author

christopherfujino commented May 8, 2021

This is the dart commit that causes this failure: https://dart.googlesource.com/sdk.git/+/3e9cdc56440bded76d8078a2567910236ad6f7b9%5E%21/#F1

Breaking change request: dart-lang/sdk#45115

cc @lrhn

@christopherfujino christopherfujino changed the title validate_ci_yaml test failing dart sdk roll breaks cocoon via transitive dep uuid_enhanced May 8, 2021
@zanderso
Copy link
Member

zanderso commented May 8, 2021

cocoon's graphql dependency is a bit old. It looks like if graphql can be updated to at least 4.1.0-beta.1, then it doesn't use uuid_enhanced anymore, and uses uuid instead, which doesn't have this problem.

/cc @CaseyHillers

I'll try to make a PR for cocoon that just bumps the graphql dependency and link it here.

@zanderso
Copy link
Member

zanderso commented May 8, 2021

After some investigation, the versions of graphql that drop the dependence on uuid_enhanced add a dependence on package:http at ^0.13.0, which conflicts with the version of package:http wanted by metrics_center, gcloud, and possibly others---I stopped looking there. I think we're stuck unless a whole slew of packages have new versions published.

@zanderso zanderso added dependency: dart Dart team may need to help us team Infra upgrades, team productivity, code health, technical debt. See also team: labels. team-infra Owned by Infrastructure team and removed team-infra Owned by Infrastructure team labels May 8, 2021
@zanderso zanderso added the P1 label May 10, 2021
@zanderso
Copy link
Member

Marking P1 since the Dart -> Engine roller is paused on this.

@CaseyHillers
Copy link
Contributor

The issue is the backend code isn't tested by customer_tests (which is intentional due to dart:mirrors). This looks like a breaking change, and I've also run into issues with getting the backend project onto package:http=^13.0.0.

I'll refactor the validation suite into a separate Dart project from the backend so it's not impacted by these dependencies and re-enable (fyi @christopherfujino )

@CaseyHillers CaseyHillers self-assigned this May 10, 2021
@christopherfujino
Copy link
Member Author

I'll refactor the validation suite into a separate Dart project from the backend so it's not impacted by these dependencies and re-enable (fyi @christopherfujino )

I'm curious how you would refactor the code to a different dart project. We'd still need the proto-generated dart files, right? Are you thinking of creating a separate dart project that would generate those same dart files?

@zanderso
Copy link
Member

@CaseyHillers What is the status of this? Do you need any help?

@CaseyHillers
Copy link
Contributor

We removed the ci_yaml builder to a check in the Cocoon backend. The CI validation is no longer bound to Dart rolls, but we'll need to ensure Cocoon deploys are green.

Remaining work to tracking Cocoon being readded to customer_tests is in #82705

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 31, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
dependency: dart Dart team may need to help us P2 Important issues not at the top of the work list team Infra upgrades, team productivity, code health, technical debt. See also team: labels. team-infra Owned by Infrastructure team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants