Skip to content

Remove .packages from the Dart SDK repo #48275

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

Closed
natebosch opened this issue Feb 1, 2022 · 22 comments
Closed

Remove .packages from the Dart SDK repo #48275

natebosch opened this issue Feb 1, 2022 · 22 comments
Assignees
Labels
area-sdk Use area-sdk for general purpose SDK issues (packaging, distribution, …). type-code-health Internal changes to our tools and workflows to make them cleaner, simpler, or more maintainable

Comments

@natebosch
Copy link
Member

I dropped a bunch of places reading this file in https://dart-review.googlesource.com/c/sdk/+/211503, but there are still some places that depend on it.

@natebosch natebosch added type-code-health Internal changes to our tools and workflows to make them cleaner, simpler, or more maintainable area-sdk Use area-sdk for general purpose SDK issues (packaging, distribution, …). labels Feb 1, 2022
copybara-service bot pushed a commit that referenced this issue Feb 2, 2022
Towards #48275

Change-Id: Ie4fe50d0d5afb04d4d3e7dde55c441e952218cc4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/231184
Auto-Submit: Nate Bosch <[email protected]>
Reviewed-by: Nicholas Shahan <[email protected]>
Commit-Queue: Nicholas Shahan <[email protected]>
copybara-service bot pushed a commit that referenced this issue Feb 3, 2022
Towards #48275

Change-Id: Idfe1de48b7f292b6c89e5b904f74c37232380c6b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/231065
Auto-Submit: Nate Bosch <[email protected]>
Reviewed-by: Kevin Moore <[email protected]>
Commit-Queue: Kevin Moore <[email protected]>
@mit-mit mit-mit changed the title Remove .packages from this repo Remove .packages from the Dart SDK repo Feb 7, 2022
@mit-mit
Copy link
Member

mit-mit commented Mar 7, 2022

@natebosch any chance we could assign this issue to you to get the rest removed the SDK repo?

@natebosch
Copy link
Member Author

Sure, I am planning to keep pushing on this as I have time.

@srawlins
Copy link
Member

I just grepped through this repo for "dotPackages", and found references in:

  • pkg/analyzer/lib/
  • pkg/front_end/lib/, pkg/front_end/test/, and pkg/front_end/test_cases/
  • pkg/front_end_server/test/
  • pkg/vm/bin/kernel_service.dart
  • tests/lib/isolate/, tests/lib_2/isolate/

@natebosch
Copy link
Member Author

I just grepped through this repo for "dotPackages"

Yeah there are still a bunch. Any help cleaning up whatever you find is great.

dotPackages as a variable name can sometimes refer to the path to .dart_tool/package_config.json since some tools were updated to understand either file with the same argument.

@scheglov
Copy link
Contributor

There are a few shared tests that use .packages, started failing here.

standalone_2/package/scenarios/both_dir_and_file/prefers_packages_file_test
standalone_2/package/scenarios/packages_file_in_parent/sub/packages_file_in_parent_test
standalone_2/package/scenarios/packages_file_only/packages_file_only_test

@mit-mit
Copy link
Member

mit-mit commented Apr 8, 2022

@sigmundch there are a bunch of search hits on .packages in /pkg/compiler/ & /pkg/dev_compiler/ -- is that something the web team can look at in Q2?

@mit-mit
Copy link
Member

mit-mit commented Apr 8, 2022

@johnniwinther there are a bunch of search hits on .packages in /pkg/front_end/ -- is that something the CFE team can look at in Q2?

@mit-mit
Copy link
Member

mit-mit commented Apr 8, 2022

@sortie I see a bunch of search hits on .packages in /tools.bots -- is that something you can help with?

@sigmundch
Copy link
Member

I think we should be able to get rid of the web uses of .packages. A big bulk of them come from the modular_test framework, which we created a few years back. That required changing how the framework works, but I made progress on that today via https://dart-review.googlesource.com/c/sdk/+/240651/3 and https://dart-review.googlesource.com/c/sdk/+/240650/2

copybara-service bot pushed a commit that referenced this issue Apr 9, 2022
Embed in the modules.yaml any extra paths needed for package dependencies
instead.

This is one step to help towards #48275

Change-Id: I22ef02b2b2327a0c798f2fea73d59c758a8bb0bb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/240651
Reviewed-by: Joshua Litt <[email protected]>
Commit-Queue: Sigmund Cherem <[email protected]>
@sortie
Copy link
Contributor

sortie commented Apr 11, 2022

@mit-mit OK, handled tools/bots in:

https://dart-review.googlesource.com/c/sdk/+/240842
https://dart-review.googlesource.com/c/sdk/+/240843

@johnniwinther
Copy link
Member

@mit-mit Have we deprecated the feature yet? I'd like to keep testing of the feature as long as we support it.

copybara-service bot pushed a commit that referenced this issue Apr 19, 2022
Bug: #48275
Change-Id: Ief3ff323726bdbb1870e989deb5372d88c3f4f3a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/240843
Auto-Submit: Jonas Termansen <[email protected]>
Reviewed-by: Alexander Thomas <[email protected]>
Commit-Queue: Alexander Thomas <[email protected]>
@mit-mit
Copy link
Member

mit-mit commented May 4, 2022

@johnniwinther sorry missed your question. Yes, the .packages file has been deprecated since Dart 2.8.

@mit-mit
Copy link
Member

mit-mit commented May 5, 2022

I'm making some progress in https://dart-review.googlesource.com/c/sdk/+/243625 on removing the generation of the .packages file. There are a lot of failures I don't understand though in the test results for patchset 3 for DDC:
https://ci.chromium.org/ui/p/dart/builders/try/ddc-linux-release-chrome-try/42906/overview

@athomas @sigmundch does this somehow ring a bell to you?

@athomas
Copy link
Member

athomas commented May 5, 2022

@mit-mit Probably something related to the path mangling around https://github.com/dart-lang/sdk/blob/main/pkg/test_runner/lib/src/test_suite.dart#L928

@sigmundch
Copy link
Member

@mit-mit Funny you ask today, just yesterday I came across the same error.

FYI @sortie also has a step to remove the use of .packages on the bots (see https://dart-review.googlesource.com/c/sdk/+/240842). That prompted me to make a similar change to yours to unblock him (https://dart-review.googlesource.com/c/sdk/+/243643), but also came across the errors on the DDC bots. I haven't yet investigated, but it's possible that the errors are not DDC specific, but more of an infra issue with the test runner or the http server it uses to launch browser tests. I'll try to check later today if I get a chance, but otherwise I'll look tomorrow.

@sigmundch
Copy link
Member

Turns out the DDC bot failures were due to an issue in the test runner. The test runner was generating an invalid html to bootstraps the ddc code after the change because it used the path of the package configuration file in its logic.

I'm attempting a new run with a fix in https://dart-review.googlesource.com/c/sdk/+/243643

@sigmundch
Copy link
Member

(and forgot to say, this turned out to be exactly what @athomas suggested initially. At first I didn't put together how that logic connected with DDC, but I landed in that same place after looking at the generated JavaScript)

copybara-service bot pushed a commit that referenced this issue May 10, 2022
.dart_tool/package_config.json is already present in each fileset.

Bug: #48275
Change-Id: Ib75fd91060f0b9eaeb1446cf65cb8678b2ddc63c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/240842
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Jonas Termansen <[email protected]>
Reviewed-by: Alexander Thomas <[email protected]>
@sortie
Copy link
Contributor

sortie commented May 10, 2022

.packages has now been removed from the bots, thanks everyone for helping that along :)

@mit-mit
Copy link
Member

mit-mit commented May 10, 2022

https://dart-review.googlesource.com/c/sdk/+/243625 has been created to stop generating the .packages file in the SDK

@mit-mit
Copy link
Member

mit-mit commented May 10, 2022

@natebosch are you aware of any other work for the present issue after /243625 lands?

@natebosch
Copy link
Member Author

If /243625 doesn't cause any issues I think that covers the vast majority, though there may be a bit more cleanup to do.

I found a few references - https://dart-review.googlesource.com/c/sdk/+/244046

I'm a little surprised that these references don't cause any issues when you stop creating the .packages file entirely...

copybara-service bot pushed a commit that referenced this issue May 11, 2022
Contributes to #48275

Change-Id: I3acb90b91b9b206b69ca0ae311aa43f5954c29a6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/243625
Reviewed-by: Nate Bosch <[email protected]>
Reviewed-by: Sigmund Cherem <[email protected]>
Commit-Queue: Michael Thomsen <[email protected]>
@mit-mit
Copy link
Member

mit-mit commented May 11, 2022

/243625 and /244046 have both landed 🚀 , so closing this issue

@mit-mit mit-mit closed this as completed May 11, 2022
copybara-service bot pushed a commit that referenced this issue May 11, 2022
Towards #48275

[email protected]

Change-Id: I38cc986937543c5e4d1f2a2cd4c7804d180a2741
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/244304
Commit-Queue: Nate Bosch <[email protected]>
Reviewed-by: Michael Thomsen <[email protected]>
Commit-Queue: Michael Thomsen <[email protected]>
Auto-Submit: Nate Bosch <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-sdk Use area-sdk for general purpose SDK issues (packaging, distribution, …). type-code-health Internal changes to our tools and workflows to make them cleaner, simpler, or more maintainable
Projects
None yet
Development

No branches or pull requests

8 participants