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

Remove support for packages directory #30384

Closed
4 tasks
kevmoo opened this issue Aug 9, 2017 · 17 comments
Closed
4 tasks

Remove support for packages directory #30384

kevmoo opened this issue Aug 9, 2017 · 17 comments
Labels
area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). P3 A lower priority bug or feature request
Milestone

Comments

@kevmoo
Copy link
Member

kevmoo commented Aug 9, 2017

Starting on the list

  • dart:io Platform.packageRoot
  • dart:isolate Isolate.packageRoot

Related:

  • Remove support from pkg:package_resolver – bug needed
  • Remove use from pkg:test - bug needed
@kevmoo kevmoo added this to the 2.0 milestone Aug 9, 2017
@kevmoo kevmoo added area-multi P1 A high priority bug; for example, a single project is unusable or has many test failures labels Aug 9, 2017
@kevmoo kevmoo self-assigned this Aug 9, 2017
@zanderso
Copy link
Member

zanderso commented Aug 9, 2017

Why is this P1?

@Stargator
Copy link
Contributor

Stargator commented Sep 3, 2017

@zanderso I believe its short for "Priority 1" ie Highest priority.

@kevmoo What is the impact of removing this directory?

@kevmoo
Copy link
Member Author

kevmoo commented Sep 13, 2017

Why is this P1?

It's cleanup that would be ideal in v2 – since it'll be easier to get away with breaking changes.

@dgrove
Copy link
Contributor

dgrove commented May 10, 2018

Any updates on this, @kevmoo ?

@kevmoo
Copy link
Member Author

kevmoo commented May 10, 2018

There was discussion in email about Fuscia needing/wanting packages directories still.

@zanderso @a-siva ??

@dgrove
Copy link
Contributor

dgrove commented May 22, 2018

@kevmoo will follow up on this to see what users would be affected.

@dgrove dgrove assigned MichaelRFairhurst and unassigned kevmoo and a-siva May 25, 2018
@dgrove
Copy link
Contributor

dgrove commented May 30, 2018

@MichaelRFairhurst any updates here?

@MichaelRFairhurst
Copy link
Contributor

I don't think I'll be able to take this on in two weeks in addition to my other work. I did ask on the design doc to confirm that we'd only need to do wave 1 in the meantime; however, even if that's true, I imagine tracking down those APIs to deprecate and publish could easily go wrong. For instance, package:analyzer has a near copy of package:package_resolver (in order to use a different file system API). I need to see if that's public, and if it is, that needs to be deprecated & published too.

@dgrove
Copy link
Contributor

dgrove commented Jun 5, 2018

Any updates on the plan here?

@MichaelRFairhurst
Copy link
Contributor

I have not yet had time to take a look, been putting all my time into #33221

dart-bot pushed a commit that referenced this issue Jun 6, 2018
Part of #30384. This was usually not a problem as most cases involve
additional visitors which rewrite this (for instance, inference).

However, in some cases (appears to be summaries only), that extra
resolution is not needed/skipped, so the non-instantiated constructors
remain.

That makes the constantValue of annotations vulnerable to this, and the
resulting DartObject types will have 'TypeParameterType's for there
typeArguments.

Instantiate the types to bounds, and get the ctor from the type rather
than the element, inside of AstRewriter in order to ensure these
constructors are instantiated.

Change-Id: I65f55feb751e139e68c774786bfbc53788d32995
Reviewed-on: https://dart-review.googlesource.com/58800
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Mike Fairhurst <[email protected]>
@MichaelRFairhurst
Copy link
Contributor

Strawman CL, which seems to have no issues internally: https://dart-review.googlesource.com/#/c/sdk/+/59100

@dgrove
Copy link
Contributor

dgrove commented Jun 14, 2018

Can this issue be closed at this point?

@kevmoo
Copy link
Member Author

kevmoo commented Jun 14, 2018 via email

@dgrove
Copy link
Contributor

dgrove commented Jun 15, 2018

Once this CL lands, I suggest that we remove the milestone. Landing the noop version of this is enough to allow the VM team to shed the underlying code.

@MichaelRFairhurst
Copy link
Contributor

Agreed -- now that https://dart-review.googlesource.com/c/sdk/+/60062 is landed, the packages/ directory won't work with or without dart 2. And the deprecations have already been published.

Its now clear -- as far as I can tell -- to be cleaned up at anyone's whim!

@whesse
Copy link
Contributor

whesse commented Jun 18, 2019

This issue is marked closed, but nobody has ever done the work described, of actually removing the APIs. Shouldn't it be reopened until somebody does?

@natebosch natebosch reopened this Jun 18, 2019
@natebosch natebosch added P3 A lower priority bug or feature request and removed P1 A high priority bug; for example, a single project is unusable or has many test failures labels Jun 18, 2019
@kevmoo kevmoo added the area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). label Jun 18, 2019
@kevmoo
Copy link
Member Author

kevmoo commented Jul 11, 2024

These are all done a long time ago!

@kevmoo kevmoo closed this as completed Jul 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). P3 A lower priority bug or feature request
Projects
None yet
Development

No branches or pull requests

8 participants