Skip to content
This repository has been archived by the owner on Aug 28, 2024. It is now read-only.

Deprecate --packages flag and add --package #370

Merged
merged 12 commits into from
Mar 29, 2022
Merged

Deprecate --packages flag and add --package #370

merged 12 commits into from
Mar 29, 2022

Conversation

liamappelbe
Copy link
Contributor

@liamappelbe liamappelbe commented Mar 10, 2022 •
edited
Loading

Add a --package flag to format_coverage, which takes the package's root directory, instead of the .package file. This flag defaults to the current working directory, so in the common case the user won't have to pass it at all. Deprecate the --packages flag.

Also, deprecate the packagesPath parameter and add packagePath instead, in HitMap.parseJson, HitMap.parseFiles, createHitmap, and parseCoverage.

Fixes dart-lang/tools#484

natebosch
natebosch reviewed Mar 14, 2022
View reviewed changes
Copy link
Contributor

@natebosch natebosch left a comment •
edited
Loading

Choose a reason for hiding this comment

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

In some places we've kept the --packages argument and use it to point to package_config.json instead of .packages.

https://pub.dev/documentation/package_config/latest/package_config.package_config/findPackageConfigUri.html

@liamappelbe
Copy link
Contributor Author

In some places we've kept the --packages argument and use it to point to package_config.json instead of .packages.

Michael thinks we should delete the flag. We can discuss it more here: dart-lang/tools#484

@liamappelbe liamappelbe marked this pull request as ready for review March 22, 2022 17:04
@coveralls
Copy link

coveralls commented Mar 22, 2022 •
edited
Loading

Coverage Status

Coverage increased (+2.5%) to 93.05% when pulling 6143820 on package_flag into 0790b22 on master.

@liamappelbe liamappelbe changed the title Swap --packages flag for --package Deprecate --packages flag and add --package Mar 22, 2022
kevmoo
kevmoo reviewed Mar 22, 2022
View reviewed changes
README.md Outdated Show resolved Hide resolved
@liamappelbe
Copy link
Contributor Author

Friendly ping @natebosch . I think this is ready to go. I've switched it from deleting the old flag/api to just deprecating it.

@liamappelbe liamappelbe merged commit 19bd6d4 into master Mar 29, 2022
@liamappelbe liamappelbe deleted the package_flag branch March 29, 2022 23:44
@cbracken cbracken mentioned this pull request May 14, 2022
Closed
@cbracken cbracken mentioned this pull request May 14, 2022
Merged
8 tasks
cbracken added a commit to cbracken/flutter that referenced this pull request May 14, 2022
@cbracken
Roll dependendencies
ae27659 
This rolls depdendencies to latest using
flutter update-packages --force-upgrade

This change includes three code changes:

* Removes charcode from the dependencies allowlist since it no longer
  appears in the transitive closure of dependencies of the flutter,
  flutter_test, flutter_driver, flutter_localizations, and
  integration_test packages.

* Uses Resolver.create instead of the deprecated Resolver constructor.
  The default Resolver constructor has been deprecated in favour of the
  static Resolver.create() factory function, which unfortunately happens
  to be async. Propagated the async-ness up the chain.

* Eliminates the use of the deprecated packagesPath parameter to
  HitMap.parseJson. This parameter was deprecated and replaced with
  packagePath in dart-archive/coverage#370 which
  was part of the overall deprecation of the .packages file in Dart
  itself dart-lang/sdk#48272. The overall goal
  being that end-user code shouldn't need to know about implementation
  details such as whether dependency information is stored in a
  .packages file or a package_info.json file, but rather use the
  package_config package to obtain the package metadata and perform
  other functions such as resolving its dependencies to filesystem
  paths. packagesPath was replaced by packagePath, which takes the path
  to the package directory itself. Internally, package:coverage then
  uses package_config to do the rest of the package/script URI
  resolution to filesystem paths.

This is a pre-update prior to updating flutter_template_images in
flutter#103739

Issue: flutter#103371
Issue: flutter#103775
@cbracken
Copy link
Contributor

@liamappelbe @natebosch looks like this was soft-breaking to Flutter (though our lint rules cause this to be hard-breaking). I've got a patch that fixes it up here:
flutter/flutter#103771

Unfortunately, applying that patch would break google3, because package:coverage appears to not have rolled to google3 yet, as a result, we're blocked on all flutter depdendency upgrades at the moment.

Is there a plan to roll this in soon?

@cbracken cbracken mentioned this pull request May 14, 2022
Closed
@cbracken
Copy link
Contributor

cbracken commented May 14, 2022 •
edited
Loading

I've temporarily marked these uses as ignore to get us unblocked on the Flutter side. Filed flutter/flutter#103830 to track. This will require some internal patches for Flutter coverage collection under Blaze as well. I've noted them in a hand-wavy way in the above-mentioned issue.

cbracken added a commit to flutter/flutter that referenced this pull request May 14, 2022
@cbracken
Roll dependendencies (#103771)
27c6cdb 
Roll dependendencies

This rolls depdendencies to latest using
flutter update-packages --force-upgrade

This change includes three code changes:

* Removes charcode from the dependencies allowlist since it no longer
  appears in the transitive closure of dependencies of the flutter,
  flutter_test, flutter_driver, flutter_localizations, and
  integration_test packages.

* Uses Resolver.create instead of the deprecated Resolver constructor.
  The default Resolver constructor has been deprecated in favour of the
  static Resolver.create() factory function, which unfortunately happens
  to be async. Propagated the async-ness up the chain.
  This change was partially reverted and the deprecation ignored in this
  patch until package:coverage can be rolled internally at Google.

* Eliminates the use of the deprecated packagesPath parameter to
  HitMap.parseJson. This parameter was deprecated and replaced with
  packagePath in dart-archive/coverage#370 which
  was part of the overall deprecation of the .packages file in Dart
  itself dart-lang/sdk#48272. The overall goal
  being that end-user code shouldn't need to know about implementation
  details such as whether dependency information is stored in a
  .packages file or a package_info.json file, but rather use the
  package_config package to obtain the package metadata and perform
  other functions such as resolving its dependencies to filesystem
  paths. packagesPath was replaced by packagePath, which takes the path
  to the package directory itself. Internally, package:coverage then
  uses package_config to do the rest of the package/script URI
  resolution to filesystem paths.
  This change was partially reverted and the deprecation ignored in this
  patch until package:coverage can be rolled internally at Google.

This is a pre-update prior to updating flutter_template_images in
#103739

Issue: #103371
Issue: #103775
Issue: #103830

When re-applying the partially-reverted changes to code coverage,
we'll need to patch host_entrypoint.dart internally to await the Future
that we'll be returning rather than a non-async value.
@cbracken cbracken mentioned this pull request May 15, 2022
Closed
@cbracken cbracken mentioned this pull request May 31, 2022
Merged
8 tasks
cbracken added a commit to flutter/flutter that referenced this pull request May 31, 2022
@cbracken
[tool] Migrate off deprecated coverage paramters (#104997)
73ae324 
In #103771, we rolled
dependencies in Flutter, which triggered an update of package:coverage
to v1.3.1. The new version includes
dart-archive/coverage#370 in which two deprecations
landed:

* The `Resolver` default constructor was deprecated and replaced with
  the `Resolver.create` static factory method, which unfortunately
  happens to be async.
* The `packagesPath` parameter to `HitMap.parseJson`, which takes the
  path to the `.packages` file of the package for which coverage is to
  be collected, was deprecated. This parameter was replaced with
  `packagePath` in dart-archive/coverage#370 which
  was part of the overall deprecation of the .packages file in Dart
  itself dart-lang/sdk#48272. The overall goal
  being that end-user code shouldn't need to know about implementation
  details such as whether dependency information is stored in a
  .packages file or a package_info.json file, but rather use the
  package_config package to obtain the package metadata and perform
  other functions such as resolving its dependencies to filesystem
  paths. packagesPath was replaced by packagePath, which takes the path
  to the package directory itself. Internally, package:coverage then
  uses package_config to do the rest of the package/script URI
  resolution to filesystem paths.

This migrates off the deprecated `packagesPath` parameter to the
replacement `packagePath` paramter.

Issue: #103830
camsim99 pushed a commit to camsim99/flutter that referenced this pull request Aug 10, 2022
@cbracken @camsim99
[tool] Migrate off deprecated coverage paramters (flutter#104997)
860c119 
In flutter#103771, we rolled
dependencies in Flutter, which triggered an update of package:coverage
to v1.3.1. The new version includes
dart-archive/coverage#370 in which two deprecations
landed:

* The `Resolver` default constructor was deprecated and replaced with
  the `Resolver.create` static factory method, which unfortunately
  happens to be async.
* The `packagesPath` parameter to `HitMap.parseJson`, which takes the
  path to the `.packages` file of the package for which coverage is to
  be collected, was deprecated. This parameter was replaced with
  `packagePath` in dart-archive/coverage#370 which
  was part of the overall deprecation of the .packages file in Dart
  itself dart-lang/sdk#48272. The overall goal
  being that end-user code shouldn't need to know about implementation
  details such as whether dependency information is stored in a
  .packages file or a package_info.json file, but rather use the
  package_config package to obtain the package metadata and perform
  other functions such as resolving its dependencies to filesystem
  paths. packagesPath was replaced by packagePath, which takes the path
  to the package directory itself. Internally, package:coverage then
  uses package_config to do the rest of the package/script URI
  resolution to filesystem paths.

This migrates off the deprecated `packagesPath` parameter to the
replacement `packagePath` paramter.

Issue: flutter#103830
mosuem pushed a commit to dart-lang/tools that referenced this pull request Aug 28, 2024
@liamappelbe
Deprecate --packages flag and add --package (dart-archive/coverage#370)
d6028e8 
* Migrate tests to package_config.json

* Swap --packages flag for --package

* Fix analysis errors

* Keep the --packages flag, but delete support for .package in resolver

* Make separate private constructor for Resolver

* Improve resolver test coverage

* Switch readme example commands from `pub` to `dart pub`

* 1.2.0 is published, so move changes to 1.3.0-dev
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kevmoo kevmoo kevmoo left review comments

@natebosch natebosch natebosch approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Deprecate --packages flag
5 participants
@liamappelbe @coveralls @cbracken @kevmoo @natebosch