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

Mirror the dash-analytics package so it can be added to DEPS #51425

Closed
keertip opened this issue Feb 15, 2023 · 17 comments
Closed

Mirror the dash-analytics package so it can be added to DEPS #51425

keertip opened this issue Feb 15, 2023 · 17 comments
Assignees
Labels
area-build Use area-build for SDK build issues. area-infrastructure Use area-infrastructure for SDK infrastructure issues, like continuous integration bot changes.
Milestone

Comments

@keertip
Copy link
Contributor

keertip commented Feb 15, 2023

The analyzer team will be using the https://github.com/dart-lang/tools/tree/main/pkgs/dash_analytics package to wire up analytics for the server.

This has to be mirrored on Gerrit so before we can add it to the DEPS file.

@keertip keertip added the area-build Use area-build for SDK build issues. label Feb 15, 2023
@keertip keertip added this to the Dart 3 beta 2 milestone Feb 15, 2023
@keertip
Copy link
Contributor Author

keertip commented Feb 15, 2023

@bwilkerson @athomas

@keertip
Copy link
Contributor Author

keertip commented Feb 15, 2023

@whesse, please reassign if needed.

@whesse
Copy link
Contributor

whesse commented Feb 16, 2023

[Edit: This is not actually difficult, I just was uncertain if there were many packages in the new repo that would also get added.]
This may be hard, because we have to mirror the entire repo and add it to deps, but that isn't a big problem.

@bwilkerson
Copy link
Member

@eliasyishak

@eliasyishak
Copy link
Contributor

Thank you for the cc @bwilkerson... let me know if there is anything I can do to help

@whesse whesse added the area-infrastructure Use area-infrastructure for SDK infrastructure issues, like continuous integration bot changes. label Feb 17, 2023
@whesse
Copy link
Contributor

whesse commented Feb 17, 2023

Note that the generate_package_config.dart script will add all packages in the tools repo to the SDK's .dart_tool/package_config.json file, once it is added to DEPS.

Right now, there is only the single package dash_analytics in the repo, so that is what we want.

@whesse
Copy link
Contributor

whesse commented Feb 17, 2023

I have added a mirror of dart-lang/tools to the dart GoB host, and it can now be added to DEPS.

I am not clear on how this new package fits within the restrictions outlined in our sdk DEPS policy - the "raising-the-quality-of-dart-packages" document, which is linked to from our instructions about setting up a mirror.

Should it be added to the package owners tracking sheet (internal) as a non-published package?

Semantically, it seems to be the same as a package added to the sdk repo in the pkg/ directory. Is it maintained in a separate repo so that it can be used by the Flutter framework without publishing it?

@whesse whesse closed this as completed Feb 17, 2023
@eliasyishak
Copy link
Contributor

Thanks for setting everything up! Yes we have decided to create this separate repo so that we can use the analytics package within any Flutter and Dart related tooling. If we decide to publish it will likely be unlisted. @devoncarew anything else I'm missing?

@bwilkerson
Copy link
Member

@eliasyishak If there's nothing else that needs to be done first (and I don't think there is), do you want to update the DEPS file to pull in the analytics package, or would you like me to do so?

@whesse
Copy link
Contributor

whesse commented Feb 17, 2023

It can work best if the DEPS file starts pulling it in, and then that change to DEPS rolls to flutter engine DEPS, before adding actual dependencies to the package in the SDK build. That way, the tip-of-tree builders already have the new package rolled in to them before it is needed.

@bwilkerson
Copy link
Member

Good to know, thanks! Sounds like we definitely want the CL to only add the package to the DEPS file, which was what I was trying to suggest anyway.

We could then start working on a CL to use the package as long as we hold off landing the latter CL until after the roll into the flutter engine DEPS.

@eliasyishak
Copy link
Contributor

eliasyishak commented Feb 17, 2023

@bwilkerson do you mind adding it to the DEPS and tagging me in the PR? I'm not sure how to update that file would like to learn how to do so. I couldn't find anything in the Wiki on updating that file at first glance

EDIT: Ah found the documentation within the DEPS file itself

@bwilkerson
Copy link
Member

I have uploaded a CL (https://dart-review.googlesource.com/c/sdk/+/283920), but it doesn't currently work because https://dart.googlesource.com/dash_analytics doesn't yet exist.

@whesse Should it exist, or do we just need to wait a bit?

@sortie
Copy link
Contributor

sortie commented Feb 17, 2023

Your repository is named tools: https://dart.googlesource.com/tools/ :)

@devoncarew
Copy link
Member

I am not clear on how this new package fits within the restrictions outlined in our sdk DEPS policy - the "raising-the-quality-of-dart-packages" document, which is linked to from our instructions about setting up a mirror.

@whesse - good question; adding a dep for this package is expected, so I think this falls within our policy.

Agree that dep'ing it in first, and letting that work through the downstream systems like the engine, before adding code references to the package makes sense.

@bwilkerson
Copy link
Member

That helps, but I still can't get it to download it to the right place. It appears to be assuming that if it's in the sdk repo then it should go into sdk/sdk/third_party, but that if it's in the tools repo then it should go into sdk/tools/third_party.

I'm obviously missing something and should let someone more familiar with the DEPS file pull this in.

@devoncarew
Copy link
Member

No worries; @bwilkerson and @whesse - I sent you this for review: https://dart-review.git.corp.google.com/c/sdk/+/283921.

copybara-service bot pushed a commit that referenced this issue Feb 17, 2023
Bug: #51425
Change-Id: Ief12c163b19b6a36121b6863b7c35ba06bc0aff3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/283921
Commit-Queue: Devon Carew <devoncarew@google.com>
Reviewed-by: William Hesse <whesse@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-build Use area-build for SDK build issues. area-infrastructure Use area-infrastructure for SDK infrastructure issues, like continuous integration bot changes.
Projects
None yet
Development

No branches or pull requests

6 participants