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

Add a "pub workspace" to the root of the engine repository #53539

Merged
merged 35 commits into from
Jul 18, 2024

Conversation

matanlurey
Copy link
Contributor

@matanlurey matanlurey commented Jun 24, 2024

... and use it in engine_tool to prove everything is working, i.e. on CI and elsewhere.

Partial work towards flutter/flutter#147883.
Supersedes and closes #51782 (which was a prototype).

See also:

I'll probably end up moving the inline docs in pubspec.yaml to a docs/*.md once that's a thing.

pubspec.yaml Outdated
# (3) MIGRATING A NON-WORKSPACE PACKAGE TO USING THE WORKSPACE
# ------------------------------------------------------------------------------
# Many packages in this repo are still using a pre-workspace style pubspec.yaml,
# either with manuallu declared `dependency_overrides` (much of ./tools) or by
Copy link
Contributor

Choose a reason for hiding this comment

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

s/manuallu/manually

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!

pubspec.yaml Show resolved Hide resolved
tools/engine_tool/pubspec.yaml Outdated Show resolved Hide resolved
@flutter-dashboard
Copy link

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 "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use 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.

# `DEPS` file: they get a "any" version constraint and are overridden in the
# `dependency_overrides` section below.
#
# While not enforced by tooling, try to keep this list in alphabetical order.
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a TODO for now, thanks!

@chinmaygarde
Copy link
Member

The issue this was blocked on has been resolved and rolled in. Does this just need a rebase?

@jtmcdole
Copy link
Contributor

jtmcdole commented Jul 1, 2024

The issue this was blocked on has been resolved and rolled in. Does this just need a rebase?

I think we're still looking at some options for overrides. See dart-lang/pub/issues/4318

pubspec.yaml Outdated
path: ./tools/pkg/engine_repo_tools
expect: any
file: any
litetest:
Copy link
Contributor

Choose a reason for hiding this comment

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

move litetest and process_fakes to dependency_overrides and the sub-packages don't need to include redundant paths.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great suggestion, done!

@matanlurey
Copy link
Contributor Author

I am still blocked on a new enough Dart SDK that supports workspaces.

Revving here: https://dart-review.googlesource.com/c/sdk/+/373903

@matanlurey matanlurey force-pushed the engine-mono-worskpace-pubspec branch from 49b2ff0 to 9f49e49 Compare July 1, 2024 23:25
copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request Jul 2, 2024
Without this, I can't use `workspace` in the Flutter engine:
flutter/engine#53539

Change-Id: I3b68f14b94f91deda329efd98042dcf63ddb80ba
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/373903
Commit-Queue: Ryan Macnak <[email protected]>
Reviewed-by: Ryan Macnak <[email protected]>
@matanlurey matanlurey force-pushed the engine-mono-worskpace-pubspec branch from 69eace8 to ff2aa36 Compare July 3, 2024 17:52
pubspec.yaml Outdated
# ------------------------------------------------------------------------------
# (1) ADDING A NEW DEPENDENCY
# ------------------------------------------------------------------------------
# Dependencies need to be added either via the DEPS file, or by checking the are
Copy link
Contributor

@navaronbracke navaronbracke Jul 4, 2024

Choose a reason for hiding this comment

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

by checking the are available seems like a typo? Should this not be by checking they are available, like in the last phrase of this paragraph?

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.

@chinmaygarde
Copy link
Member

The linked pull request was merged. Are you still blocked on this?

@matanlurey
Copy link
Contributor Author

The linked pull request was merged. Are you still blocked on this?

Nope, looks like I can rebase.

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

auto-submit bot commented Jul 17, 2024

auto label is removed for flutter/engine/53539, due to - The status or check suite Mac mac_host_engine has failed. Please fix the issues identified (or deflake) before re-applying this label.

  • The status or check suite Linux linux_host_engine has failed. Please fix the issues identified (or deflake) before re-applying this label.

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

auto-submit bot commented Jul 17, 2024

auto label is removed for flutter/engine/53539, due to - The status or check suite Linux linux_unopt 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 17, 2024
Copy link
Contributor

@jtmcdole jtmcdole left a comment

Choose a reason for hiding this comment

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

still lgtm

Copy link
Contributor

auto-submit bot commented Jul 17, 2024

auto label is removed for flutter/engine/53539, due to - The status or check suite Linux linux_unopt has failed. Please fix the issues identified (or deflake) before re-applying this label.

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

auto-submit bot commented Jul 17, 2024

auto label is removed for flutter/engine/53539, due to - The status or check suite Linux linux_unopt has failed. Please fix the issues identified (or deflake) before re-applying this label.

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jul 17, 2024
@matanlurey matanlurey added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 18, 2024
@auto-submit auto-submit bot merged commit 8bcf638 into flutter:main Jul 18, 2024
33 checks passed
@matanlurey matanlurey deleted the engine-mono-worskpace-pubspec branch July 18, 2024 16:19
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jul 18, 2024
…151977)

flutter/engine@d58ba74...8bcf638

2024-07-18 [email protected] Add a "pub workspace" to the root of the engine repository (flutter/engine#53539)
2024-07-18 [email protected] Roll Skia from 9d391088f52c to 939e1dac9815 (1 revision) (flutter/engine#53990)

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

To file a bug in Flutter: 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
itsjustkevin pushed a commit to itsjustkevin/engine that referenced this pull request Jul 19, 2024
…3539)

... and use it in `engine_tool` to prove everything is working, i.e. on CI and elsewhere.

Partial work towards flutter/flutter#147883.
Supersedes and closes flutter#51782 (which was a prototype).

See also:
- https://flutter.dev/go/pub-workspace, the design doc on the feature.
- dart-lang/pub-dev#7762, an example PR.

I'll probably end up moving the inline docs in `pubspec.yaml` to a `docs/*.md` once that's a thing.
itsjustkevin added a commit to itsjustkevin/engine that referenced this pull request Jul 19, 2024
TytaniumDev pushed a commit to TytaniumDev/flutter that referenced this pull request Aug 7, 2024
…lutter#151977)

flutter/engine@d58ba74...8bcf638

2024-07-18 [email protected] Add a "pub workspace" to the root of the engine repository (flutter/engine#53539)
2024-07-18 [email protected] Roll Skia from 9d391088f52c to 939e1dac9815 (1 revision) (flutter/engine#53990)

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

To file a bug in Flutter: 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
Buchimi pushed a commit to Buchimi/flutter that referenced this pull request Sep 2, 2024
…lutter#151977)

flutter/engine@d58ba74...8bcf638

2024-07-18 [email protected] Add a "pub workspace" to the root of the engine repository (flutter/engine#53539)
2024-07-18 [email protected] Roll Skia from 9d391088f52c to 939e1dac9815 (1 revision) (flutter/engine#53990)

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

To file a bug in Flutter: 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
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants